Skip to content
  • Jamey Sharp's avatar
    Move XID and sync handling from SyncHandle to LockDisplay to fix races. · a6d974dc
    Jamey Sharp authored
    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: default avatarJamey Sharp <jamey@minilop.net>
    Signed-off-by: default avatarJosh Triplett <josh@freedesktop.org>
    a6d974dc