Bug 1753 - Crash with Xfixes events on client closedown
Summary: Crash with Xfixes events on client closedown
Status: CLOSED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/General (show other bugs)
Version: unspecified
Hardware: x86 (IA32) Linux (All)
: high major
Assignee: Keith Packard
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-11-01 08:50 UTC by Owen Taylor
Modified: 2011-10-15 17:21 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to fix (1.57 KB, patch)
2004-11-01 08:51 UTC, Owen Taylor
no flags Details | Splinter Review
Patch in the style of TryClientEvents (1.12 KB, patch)
2004-11-01 10:32 UTC, Keith Packard
no flags Details | Splinter Review

Description Owen Taylor 2004-11-01 08:50:10 UTC
This should be an evocative stacktrace:

#0  WriteToClient (who=0x9f73460, count=32, buf=0xfef082d0 "H") at io.c:898
#1  0x08063eeb in WriteEventsToClient (pClient=0x9f73460, count=32,
    events=0xfef082d0) at events.c:4485
#2  0x080dd577 in CursorDisplayCursor (pScreen=0x9e866f8, pCursor=0x9f69b48)
    at cursor.c:108
#3  0x080da158 in AnimCurDisplayCursor (pScreen=0x9e866f8, pCursor=0x9f69b48)
    at animcur.c:234
#4  0x08062fcb in ChangeToCursor (cursor=0x9f69b48) at events.c:811
#5  0x08065270 in CheckMotion (xE=0x0) at events.c:2006
#6  0x08050f7b in UnmapWindow (pWin=0x9f21c98, fromConfigure=0)
    at window.c:3048
#7  0x080e35ee in compFreeClientWindow (pWin=0x9f21c98, id=1077936131)
    at compalloc.c:170
#8  0x080e0dc1 in FreeCompositeClientWindow (value=0x9f21c98, ccwid=1077936131)
    at compext.c:65
#9  0x0804cfee in FreeResource (id=1077936131, skipDeleteFuncType=0)
    at resource.c:507
#10 0x080e2fac in compUnredirectWindow (pClient=0x9f73460, pWin=0x0, update=0)
    at compalloc.c:215
#11 0x080e320d in compFreeClientSubwindows (pWin=0x9e8b770, id=0)
    at compalloc.c:338
#12 0x080e0dd9 in FreeCompositeClientSubwindows (value=0x9e8b770,
    ccwid=1077936129) at compext.c:74
#13 0x0804d53a in FreeClientResources (client=0x9f73460) at resource.c:753
#14 0x0805e51e in CloseDownClient (client=0x9f73460) at dispatch.c:3653
#15 0x0805ef78 in Dispatch () at dispatch.c:434
#16 0x0804bc0d in main (argc=3, argv=0xfef08af4, envp=0xfef08b04) at main.c:430

When a compositing manager selecting for cursor notify events closes,
you get this reentrancy problem -- WriteToClient is crashing because
who->osPrivate == NULL. I tend to think that the generation of
events here is likely a bit bogus - the compositing manager actually
hasn't created any windows so the UnmapWindow isn't really a change
to window structure. But the compositing manager *could* have
created windows, so the reentrency problem should be fixed as such.

The standard seems to simply insert a check on client->clientGone.
(Probably any call to WriteEventsToClient() in the server without
such a check is a bug... might be a worthwhile cleanup project.)

Here's a patch for both cursor and selection events for xfixes.
This needs to be applied both to xorg-x11 and to xserver.
(Was a lot more fun to debug with Xfake then with a real server)
Comment 1 Owen Taylor 2004-11-01 08:51:30 UTC
Created attachment 1202 [details] [review]
Patch to fix
Comment 2 Keith Packard 2004-11-01 10:32:03 UTC
The UnmapWindow happens when the window is unredirected -- it works like
reparent in that the application window is automatically unmapped and remapped.

Aside from that, I think I prefer to check clientGone in the mask test, not for
any performance reasons, but just to make the code look more like the typical
DIX event distribution code (c.f. TryClientEvents).
Comment 3 Keith Packard 2004-11-01 10:32:45 UTC
Created attachment 1204 [details] [review]
Patch in the style of TryClientEvents
Comment 4 Owen Taylor 2004-11-01 10:45:46 UTC
Sure, you know better :-)

Though I'd stylistically prefer () around 

e->eventMask & XFixesDisplayCursorNotifyMask

in the first case as well. While that particular case is fine, the
unexpectedly low precedence of & always makes me nervous about 
unparenthesized usage.
Comment 5 Tomas Carnecky 2007-02-01 05:37:41 UTC
This bus still exists in xorg-server-1.2.0, see http://lists.freedesktop.org/archives/xorg/2007-January/021368.html. Can someone please apply this patch? Or was it already fixed in another way?
Comment 6 Keith Packard 2007-02-19 15:35:20 UTC
Thanks for the reminder. I've patched master and also cherry-picked to server-1.3-branch


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.