From 010b713c1af81ba1b323daa6732f9d1eec63e66c Mon Sep 17 00:00:00 2001 From: Paul Gofman Date: Tue, 2 Dec 2025 11:25:18 -0600 Subject: [PATCH] xcb_io: use xcb_take_socket2 if available. That fixes the deadlock that may happen when a XCB connection is concurretly used outside of libx11 and there is user display lock taken. --- configure.ac | 2 ++ src/Xxcbint.h | 2 ++ src/locking.c | 37 +++++++++++++++++++++++++++++++------ src/locking.h | 13 ++++++++++--- src/xcb_io.c | 40 ++++++++++++++++++++++++++++++++++++++-- 5 files changed, 83 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index fe2b7669..7aba4885 100644 --- a/configure.ac +++ b/configure.ac @@ -348,6 +348,8 @@ AC_SUBST(X11_LIBDIR) PKG_CHECK_MODULES(X11, [$X11_REQUIRES]) X11_CFLAGS="$X11_CFLAGS $XTHREAD_CFLAGS" +AC_CHECK_LIB([xcb], [xcb_take_socket2], [AC_DEFINE([HAVE_XCB_TAKE_SOCKET2], [1], [using xcb_take_socket2])],,[$X11_LIBS]) + # # Yes, it would be nice to put the locale data in # /usr/share, but the locale stuff includes loadable diff --git a/src/Xxcbint.h b/src/Xxcbint.h index d8293259..4e8aa4ca 100644 --- a/src/Xxcbint.h +++ b/src/Xxcbint.h @@ -40,6 +40,8 @@ typedef struct _X11XCBPrivate { xcondition_t event_notify; int event_waiter; xcondition_t reply_notify; + + uint32_t take_socket_seq; } _X11XCBPrivate; /* xcb_disp.c */ diff --git a/src/locking.c b/src/locking.c index 3625bd27..f15b41f9 100644 --- a/src/locking.c +++ b/src/locking.c @@ -69,6 +69,27 @@ extern LockInfoPtr _Xi18n_lock; /* in lcConv.c */ extern LockInfoPtr _conv_lock; +static void lock_dpy_mutex( + Display *dpy + ) +{ + xmutex_lock(dpy->lock->mutex); + dpy->lock->mutex_lock_thread = xthread_self(); +} + +static void unlock_dpy_mutex( + Display *dpy) +{ + dpy->lock->mutex_lock_thread = 0; + xmutex_unlock(dpy->lock->mutex); +} + +static int _XIsDisplayLockedByCurrentThread( + struct _XLockInfo *lock) +{ + return lock->mutex_lock_thread == xthread_self(); +} + #ifdef WIN32 static DWORD _X_TlsIndex = (DWORD)-1; @@ -176,7 +197,7 @@ static void _XLockDisplayWarn( #endif /* XTHREADS_DEBUG */ } - xmutex_lock(dpy->lock->mutex); + lock_dpy_mutex(dpy); if (strcmp(file, "XlibInt.c") == 0) { if (!xlibint_unlock) @@ -242,7 +263,7 @@ static void _XUnlockDisplay( #endif /* XTHREADS_WARN */ if (dpy->in_ifevent == 0 || !xthread_equal(dpy->ifevent_thread, xthread_self())) - xmutex_unlock(dpy->lock->mutex); + unlock_dpy_mutex(dpy); } @@ -333,7 +354,7 @@ static void _XPopReader( static void _XConditionWait( xcondition_t cv, - xmutex_t mutex + struct _XLockInfo *lock XTHREADS_FILE_LINE_ARGS ) { @@ -356,7 +377,9 @@ static void _XConditionWait( lock_hist_loc = 0; #endif /* XTHREADS_WARN */ - xcondition_wait(cv, mutex); + xcondition_wait(cv, lock->mutex); + lock->mutex_lock_thread = xthread_self(); + #ifdef XTHREADS_WARN locking_thread = self; @@ -467,7 +490,7 @@ static void _XLockDisplay( #ifdef XTHREADS_WARN _XLockDisplayWarn(dpy, file, line); #else - xmutex_lock(dpy->lock->mutex); + lock_dpy_mutex(dpy); #endif if (dpy->lock->locking_level > 0) @@ -501,7 +524,7 @@ static void _XInternalLockDisplay( #ifdef XTHREADS_WARN _XLockDisplayWarn(dpy, file, line); #else - xmutex_lock(dpy->lock->mutex); + lock_dpy_mutex(dpy); #endif if (!wskip && dpy->lock->locking_level > 0) _XDisplayLockWait(dpy); @@ -566,6 +589,7 @@ static int _XInitDisplayLock( xthread_clear_id(dpy->lock->reading_thread); xthread_clear_id(dpy->lock->conni_thread); xmutex_init(dpy->lock->mutex); + dpy->lock->mutex_lock_thread = 0; xmutex_set_name(dpy->lock->mutex, "Xlib Display"); xcondition_init(dpy->lock->cv); xcondition_set_name(dpy->lock->cv, "XLockDisplay"); @@ -582,6 +606,7 @@ static int _XInitDisplayLock( dpy->lock->condition_signal = _XConditionSignal; dpy->lock->condition_broadcast = _XConditionBroadcast; dpy->lock->create_cvl = _XCreateCVL; + dpy->lock->in_display_lock = _XIsDisplayLockedByCurrentThread; dpy->lock->lock_wait = NULL; /* filled in by XLockDisplay() */ return 0; diff --git a/src/locking.h b/src/locking.h index 59fc866e..0b7571b3 100644 --- a/src/locking.h +++ b/src/locking.h @@ -54,6 +54,7 @@ extern xthread_t (*_Xthread_self_fn)( /* in XlibInt.c */ struct _XLockInfo { xmutex_t mutex; /* mutex for critical sections */ + xthread_t mutex_lock_thread; /* thread which holds mutex */ int reply_bytes_left; /* nbytes of the reply still to read */ Bool reply_was_read; /* _XReadEvents read a reply for _XReply */ struct _XCVList *reply_awaiters; /* list of CVs for _XReply */ @@ -82,7 +83,7 @@ struct _XLockInfo { ); void (*condition_wait)( xcondition_t /* cv */, - xmutex_t /* mutex */ + struct _XLockInfo */* mutex data */ #if defined(XTHREADS_WARN) || defined(XTHREADS_FILE_LINE) , char* /* file */, int /* line */ @@ -124,6 +125,10 @@ struct _XLockInfo { struct _XCVList *(*create_cvl)( Display * /* dpy */ ); + /* used in xcb_io.c */ + int (*in_display_lock)( + struct _XLockInfo */* mutex data */ + ); }; #define UnlockNextEventReader(d) if ((d)->lock) \ @@ -131,20 +136,22 @@ struct _XLockInfo { #if defined(XTHREADS_WARN) || defined(XTHREADS_FILE_LINE) #define ConditionWait(d,c) if ((d)->lock) \ - (*(d)->lock->condition_wait)(c, (d)->lock->mutex,__FILE__,__LINE__) + (*(d)->lock->condition_wait)(c, (d)->lock,__FILE__,__LINE__) #define ConditionSignal(d,c) if ((d)->lock) \ (*(d)->lock->condition_signal)(c,__FILE__,__LINE__) #define ConditionBroadcast(d,c) if ((d)->lock) \ (*(d)->lock->condition_broadcast)(c,__FILE__,__LINE__) #else #define ConditionWait(d,c) if ((d)->lock) \ - (*(d)->lock->condition_wait)(c, (d)->lock->mutex) + (*(d)->lock->condition_wait)(c, (d)->lock) #define ConditionSignal(d,c) if ((d)->lock) \ (*(d)->lock->condition_signal)(c) #define ConditionBroadcast(d,c) if ((d)->lock) \ (*(d)->lock->condition_broadcast)(c) #endif +#define IsDisplayLockedByCurrentThread(d) (((d)->lock) ? (*(d)->lock->in_display_lock)((d)->lock) : 0) + typedef struct _LockInfoRec { xmutex_t lock; } LockInfoRec; diff --git a/src/xcb_io.c b/src/xcb_io.c index 17ac8005..776f7ab5 100644 --- a/src/xcb_io.c +++ b/src/xcb_io.c @@ -59,15 +59,52 @@ xcb_fail_assert(_message, _var); \ } while (0) +#ifdef HAVE_XCB_TAKE_SOCKET2 +static void return_socket(void *closure, uint32_t socket_seq) +{ + Display *dpy = closure; + int display_locked; + /* When XCB_TAKE_SOCKET_THREAD_SAFE_CALLBACK flag is used it may happen that return_socket is called for + * already returned socket once again, in particular, during our call to xcb_take_socket2() when we already + * hold display lock. */ + display_locked = IsDisplayLockedByCurrentThread(dpy); + if(!display_locked) + InternalLockDisplay(dpy, /* don't skip user locks */ 0); + if(dpy->xcb->take_socket_seq == socket_seq && dpy->bufmax != dpy->buffer) + { + _XSend(dpy, NULL, 0); + dpy->bufmax = dpy->buffer; + } + if(!display_locked) + UnlockDisplay(dpy); +} + +static int take_socket(Display *dpy, int flags, uint64_t *sent) +{ + xcb_take_socket_params_t p; + + p.return_socket_callback.return_socket = return_socket; + p.closure = dpy; + p.flags = XCB_TAKE_SOCKET_THREAD_SAFE_CALLBACK; + return xcb_take_socket2(dpy->xcb->connection, flags, sent, &p, &dpy->xcb->take_socket_seq); +} +#else static void return_socket(void *closure) { Display *dpy = closure; + InternalLockDisplay(dpy, /* don't skip user locks */ 0); _XSend(dpy, NULL, 0); dpy->bufmax = dpy->buffer; UnlockDisplay(dpy); } +static int take_socket(Display *dpy, int flags, uint64_t *sent) +{ + return xcb_take_socket(dpy->xcb->connection, return_socket, dpy, flags, sent); +} +#endif + static Bool require_socket(Display *dpy) { if(dpy->bufmax == dpy->buffer) @@ -78,8 +115,7 @@ static Bool require_socket(Display *dpy) * to set our errors aside for us. */ if(dpy->xcb->event_owner != XlibOwnsEventQueue) flags = XCB_REQUEST_CHECKED; - if(!xcb_take_socket(dpy->xcb->connection, return_socket, dpy, - flags, &sent)) { + if(!take_socket(dpy, flags, &sent)) { _XIOError(dpy); return False; }