Bug 30032

Created attachment 38453 [details] [review]
"git diff" patch against master

Hi,

Xorg restarted each session after some activity (20-30 minutes).

Here is a simple fixup in "dixutils.c:_CallCallbacks"

- by moving recursion count check *before* list iteration and actual callbacks' invocation;

- can say for sure, though, whether counters (member `int inCallback' from "include/dixstruct.h:171:CallbackListRec" structure) were supposed to grow greater than 1 before iteration,

- so cannot ascertain whether the new behavior is required.


There is a gdb backtrace top:

#104774  RecordFlushAllContexts (pcbl=0x878748, nulldata=0x0, calldata=0x0) at record.c:867
#104775  0x000000000043a9d2 in _CallCallbacks (pcbl=0x878748, call_data=0x0) at dixutils.c:745
#104776  0x000000000043adb6 in CallCallbacks (pcbl=0x878748, call_data=0x0) at dixutils.c:877
#104777  0x000000000047e5fd in FlushAllOutput () at io.c:614
#104778  0x000000000042c219 in Dispatch () at dispatch.c:453
#104779  0x000000000042461e in main (argc=18, argv=0x7fff83be4828, envp=0x7fff83be48c0) at main.c:291


There were some about 104780 stack calls between these lines of code:

a. dix/main.c:main:291 calls Dispatch();

b. dix/dispatch.c:Dispatch:543 calls FlushAllOutput();

c. os/io.c:FlushAllOutput:614 triggers CallCallbacks(&FlushCallback, NULL);

d. dix/dixutils.c:CallCallbacks:877 falls through _CallCallbacks(pcbl, call_data);

e. dix/dixutils.c:_CallCallbacks:743 dispatches (*(cbr->proc)) (pcbl, cbr->data, call_data);

f. record/record.c:RecordFlushAllContexts:867 entails RecordFlushReplyBuffer(ppAllContexts[eci], NULL, 0, NULL, 0);

g. record/record.c:RecordFlushReplyBuffer:251 enters WriteToClient;

h. os/io.c:WriteToClient:824: triggers, as in c, again CallCallbacks(&FlushCallback, NULL);

i. dix/dixutils.c:CallCallbacks:877 back (as in d) to _CallCallbacks(pcbl, call_data);


So I changed "dix/dixutils.c" lines 732-747

- from:

<<<
static void 
_CallCallbacks(
    CallbackListPtr    *pcbl,
    pointer	    call_data)
{
    CallbackListPtr cbl = *pcbl;
    CallbackPtr     cbr, pcbr;

    ++(cbl->inCallback);
    for (cbr = cbl->list; cbr != NULL; cbr = cbr->next)
    {
	(*(cbr->proc)) (pcbl, cbr->data, call_data);
    }
    --(cbl->inCallback);

    if (cbl->inCallback) return;
>>>

- into:

<<<
static void 
_CallCallbacks(
    CallbackListPtr    *pcbl,
    pointer	    call_data)
{
    CallbackListPtr cbl = *pcbl;
    CallbackPtr     cbr, pcbr;

    if (cbl->inCallback) return;

    ++(cbl->inCallback);
    for (cbr = cbl->list; cbr != NULL; cbr = cbr->next)
    {
	(*(cbr->proc)) (pcbl, cbr->data, call_data);
    }
    --(cbl->inCallback);
>>>


So 6 is the number of the mutual recursive functions' chain:

1. dix/dixutils.c:CallCallbacks:877
2. dix/dixutils.c:_CallCallbacks:743
3. record/record.c:RecordFlushAllContexts:867
4. record/record.c:RecordFlushReplyBuffer:251
5. os/io.c:WriteToClient:824


In "_CallCallbacks", the value of the `cbl->inCallback' variable was about 20000 at the bottom of the stack, which verifies: 20000 * 5 ~= 105000.


An earlier, 29.08.2010 date, commit might have triggered it:
http://cgit.freedesktop.org/xorg/xserver/commit/?id=c65f610e12f9df168d5639534ed3c2bd40afffc8


It seems to occur not with xf86-video-nouveau, besides xf86-video-intel.


Thanks,
s.