Support multiple independent internal sync handlers

Xlib has several independent tasks that need to be performed with the
display unlocked. It does this by replacing the existing sync handler with
one of a variety of internal sync handlers. However, if multiple internal
sync handlers need to run, then the last one registering wins and
previously registered internal sync handlers are never invoked. This
manifested as a bug with DRI applications on Xlib/XCB as that requires
both an XID handler after every XID allocation, and the periodic sequence
number handler. The XID handler would win, and the sequence number handler
would never be invoked.

Fix this by unifying the internal sync handler mechanism into a single
function that calls all of the known internal sync handlers. They all need
to deal with being called when not strictly necessary now.

Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Jamey Sharp <jamey@minilop.net>
Signed-off-by: Josh Triplett <josh@freedesktop.org>

include/X11/Xlibint.h

@@ -959,9 +959,6 @@ extern void _XGetAsyncData(
     int		/* datalen */,
     int		/* discardtotal */
 );
-extern void _XSetSeqSyncFunction(
-    Display*	/* dpy */
-);
 extern void _XFlush(
     Display*	/* dpy */
 );

src/XlibInt.c

@@ -42,11 +42,13 @@ from The Open Group.
 #include <config.h>
 #endif
 #include "Xlibint.h"
+#include "Xprivate.h"
 #include <X11/Xpoll.h>
 #if !USE_XCB
 #include <X11/Xtrans/Xtrans.h>
 #include <X11/extensions/xcmiscstr.h>
 #endif /* !USE_XCB */
+#include <assert.h>
 #include <stdio.h>
 #ifdef WIN32
 #include <direct.h>
@@ -583,28 +585,42 @@ int _XSeqSyncFunction(
 	GetEmptyReq(GetInputFocus, req);
 	(void) _XReply (dpy, (xReply *)&rep, 0, xTrue);
 	sent_sync = 1;
-    }
-    /* could get XID handler while waiting for reply in MT env */
-    if (dpy->synchandler == _XSeqSyncFunction && !sync_hazard(dpy)) {
-	dpy->synchandler = dpy->savedsynchandler;
-	dpy->flags &= ~XlibDisplayPrivSync;
-    }
+    } else if (sync_hazard(dpy))
+	_XSetPrivSyncFunction(dpy);
     UnlockDisplay(dpy);
     if (sent_sync)
         SyncHandle();
     return 0;
 }
 
-void _XSetSeqSyncFunction(
-    register Display *dpy)
+static int
+_XPrivSyncFunction (Display *dpy)
 {
-    if (!(dpy->flags & XlibDisplayPrivSync) && sync_hazard(dpy)) {
+    assert(dpy->synchandler == _XPrivSyncFunction);
+    assert((dpy->flags & XlibDisplayPrivSync) != 0);
+    dpy->synchandler = dpy->savedsynchandler;
+    dpy->savedsynchandler = NULL;
+    dpy->flags &= ~XlibDisplayPrivSync;
+    _XIDHandler(dpy);
+    _XSeqSyncFunction(dpy);
+    return 0;
+}
+
+void _XSetPrivSyncFunction(Display *dpy)
+{
+    if (!(dpy->flags & XlibDisplayPrivSync)) {
 	dpy->savedsynchandler = dpy->synchandler;
-	dpy->synchandler = _XSeqSyncFunction;
+	dpy->synchandler = _XPrivSyncFunction;
 	dpy->flags |= XlibDisplayPrivSync;
     }
 }
 
+void _XSetSeqSyncFunction(Display *dpy)
+{
+    if (sync_hazard(dpy))
+	_XSetPrivSyncFunction (dpy);
+}
+
 #if !USE_XCB
 #ifdef XTHREADS
 static void _XFlushInt(
@@ -1526,34 +1542,35 @@ _XGetMiscCode(
     }
 }
 
-static int
+int
 _XIDHandler(
     register Display *dpy)
 {
     xXCMiscGetXIDRangeReply grep;
     register xXCMiscGetXIDRangeReq *greq;
+    int sent_req = 0;
 
     LockDisplay(dpy);
-    _XGetMiscCode(dpy);
-    if (dpy->xcmisc_opcode > 0) {
-	GetReq(XCMiscGetXIDRange, greq);
-	greq->reqType = dpy->xcmisc_opcode;
-	greq->miscReqType = X_XCMiscGetXIDRange;
-	if (_XReply (dpy, (xReply *)&grep, 0, xTrue) && grep.count) {
-	    dpy->resource_id = ((grep.start_id - dpy->resource_base) >>
-				dpy->resource_shift);
-	    dpy->resource_max = dpy->resource_id;
-	    if (grep.count > 5)
-		dpy->resource_max += grep.count - 6;
-	    dpy->resource_max <<= dpy->resource_shift;
+    if (dpy->resource_max == dpy->resource_mask + 1) {
+	_XGetMiscCode(dpy);
+	if (dpy->xcmisc_opcode > 0) {
+	    GetReq(XCMiscGetXIDRange, greq);
+	    greq->reqType = dpy->xcmisc_opcode;
+	    greq->miscReqType = X_XCMiscGetXIDRange;
+	    if (_XReply (dpy, (xReply *)&grep, 0, xTrue) && grep.count) {
+		dpy->resource_id = ((grep.start_id - dpy->resource_base) >>
+				    dpy->resource_shift);
+		dpy->resource_max = dpy->resource_id;
+		if (grep.count > 5)
+		    dpy->resource_max += grep.count - 6;
+		dpy->resource_max <<= dpy->resource_shift;
+	    }
+	    sent_req = 1;
 	}
     }
-    if (dpy->flags & XlibDisplayPrivSync) {
-	dpy->synchandler = dpy->savedsynchandler;
-	dpy->flags &= ~XlibDisplayPrivSync;
-    }
     UnlockDisplay(dpy);
-    SyncHandle();
+    if (sent_req)
+	SyncHandle();
     return 0;
 }
 
@@ -1567,11 +1584,7 @@ XID _XAllocID(
 
    id = dpy->resource_id << dpy->resource_shift;
    if (id >= dpy->resource_max) {
-       if (!(dpy->flags & XlibDisplayPrivSync)) {
-	   dpy->savedsynchandler = dpy->synchandler;
-	   dpy->flags |= XlibDisplayPrivSync;
-       }
-       dpy->synchandler = _XIDHandler;
+	_XSetPrivSyncFunction(dpy);
        dpy->resource_max = dpy->resource_mask + 1;
    }
    if (id <= dpy->resource_mask) {
@@ -1627,11 +1640,7 @@ void _XAllocIDs(
 		    dpy->resource_id = id;
 	    }
 	    if (id >= dpy->resource_max) {
-		if (!(dpy->flags & XlibDisplayPrivSync)) {
-		    dpy->savedsynchandler = dpy->synchandler;
-		    dpy->flags |= XlibDisplayPrivSync;
-		}
-		dpy->synchandler = _XIDHandler;
+		_XSetPrivSyncFunction(dpy);
 		dpy->resource_max = dpy->resource_mask + 1;
 	    }
 	}

src/Xprivate.h

@@ -0,0 +1,15 @@
+/* Copyright (C) 2008 Jamey Sharp, Josh Triplett
+ * This file is licensed under the MIT license. See the file COPYING.
+ *
+ * As Xlibint.h has long become effectively public API, this header exists
+ * for new private functions that nothing outside of libX11 should call.
+ */
+
+#ifndef XPRIVATE_H
+#define XPRIVATE_H
+
+extern int _XIDHandler(Display *dpy);
+extern void _XSetPrivSyncFunction(Display *dpy);
+extern void _XSetSeqSyncFunction(Display *dpy);
+
+#endif /* XPRIVATE_H */

src/xcb_io.c

@@ -3,6 +3,7 @@
 
 #include "Xlibint.h"
 #include "locking.h"
+#include "Xprivate.h"
 #include "Xxcbint.h"
 #include <xcb/xcbext.h>
 
@@ -350,43 +351,37 @@ void _XFlush(Display *dpy)
 	_XEventsQueued(dpy, QueuedAfterReading);
 }
 
-static int
-_XIDHandler(Display *dpy)
+static const XID inval_id = ~0UL;
+
+int _XIDHandler(Display *dpy)
 {
-	XID next = xcb_generate_id(dpy->xcb->connection);
+	XID next;
+
+	if (dpy->xcb->next_xid != inval_id)
+	    return 0;
+
+	next = xcb_generate_id(dpy->xcb->connection);
 	LockDisplay(dpy);
+	dpy->xcb->next_xid = next;
 #ifdef XTHREADS
 	if (dpy->lock)
 		(*dpy->lock->user_unlock_display)(dpy);
 #endif
-	dpy->xcb->next_xid = next;
-	if(dpy->flags & XlibDisplayPrivSync)
-	{
-		dpy->synchandler = dpy->savedsynchandler;
-		dpy->flags &= ~XlibDisplayPrivSync;
-	}
 	UnlockDisplay(dpy);
-	SyncHandle();
 	return 0;
 }
 
 /* _XAllocID - resource ID allocation routine. */
 XID _XAllocID(Display *dpy)
 {
-	const XID inval = ~0UL;
 	XID ret = dpy->xcb->next_xid;
+	assert (ret != inval_id);
 #ifdef XTHREADS
-	if (ret != inval && dpy->lock)
+	if (dpy->lock)
 		(*dpy->lock->user_lock_display)(dpy);
 #endif
-	dpy->xcb->next_xid = inval;
-
-	if(!(dpy->flags & XlibDisplayPrivSync))
-	{
-		dpy->savedsynchandler = dpy->synchandler;
-		dpy->flags |= XlibDisplayPrivSync;
-	}
-	dpy->synchandler = _XIDHandler;
+	dpy->xcb->next_xid = inval_id;
+	_XSetPrivSyncFunction(dpy);
 	return ret;
 }