[Xcb] [Bug 23192] racy access to dpy->synchandler in libX11 >= 1.1.99.2

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=23192

--- Comment #6 from Josh Triplett <josh at freedesktop.org> 2010-04-15 13:44:40 PDT ---
We just pushed the following fix:

commit a6d974dc59f2722b36e2df9d4f07aeee4f83ce43
Author: Jamey Sharp <jamey at minilop.net>
Date:   Thu Apr 15 13:05:08 2010 -0700

    Move XID and sync handling from SyncHandle to LockDisplay to fix races.

    XID and sync handling happened via _XPrivSyncHandler, assigned to
    dpy->synchandler and called from SyncHandle.  _XPrivSyncHandler thus ran
    without the Display lock, so manipulating the Display caused races, and
    these races led to assertions in multithreaded code (demonstrated via
    ico).

    In the XTHREADS case, after you've called XInitThreads, we can hook
    LockDisplay and UnlockDisplay.  Use that to run _XIDHandler and
    _XSeqSyncHandler from LockDisplay rather than SyncHandle; we then know
    that we hold the lock, and thus we can avoid races.  We think it makes
    sense to do these both from LockDisplay rather than UnlockDisplay, so
    that you know you have valid sync and a valid XID before you start
    setting up the request you locked to prepare.

    In the !XTHREADS case, or if you haven't called XInitThreads, you don't
    get to use Xlib from multiple threads, so we can use the logic we have
    now (with synchandler and savedsynchandler) without any concern about
    races.

    This approach gets a bit exciting when the XID and sequence sync
    handlers drop and re-acquire the Display lock. Reacquisition will re-run
    the handlers, but they return immediately unless they have work to do,
    so they can't recurse more than once.  In the worst case, if both of
    them have work to do, we can nest the Display lock three deep.  In the
    case of the _XIDHandler, we drop the lock to call xcb_generate_id, which
    takes the socket back if it needs to request more XIDs, and taking the
    socket back will reacquire the lock; we take care to avoid letting
    _XIDHandler run again and re-enter XCB from the return_socket callback
    (which causes Very Bad Things, and is Not Allowed).

    Tested with ico (with 1 and 20 threads), and with several test programs
    for XID and sequence sync.  Tested with and without XInitThreads(), and
    with and without XCB.

    Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=23192

    Signed-off-by: Jamey Sharp <jamey at minilop.net>
    Signed-off-by: Josh Triplett <josh at freedesktop.org>

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.