[PATCH v3 libX11 3/3] config: Add --enable-checked-locks to catch missing XInitThreads calls.
Rami Ylimäki
rami.ylimaki at vincit.fi
Fri Feb 4 06:42:31 PST 2011
This change makes it possible to detect missing XInitThreads calls in
X clients. If a client hasn't called XInitThreads and enters a
critical section in Xlib concurrently, the client will be aborted with
a message informing about the missing call.
Enabling thread safety by default causes a minor performance
regression for single threaded X clients. However, the performance
regression is only noticeable in the worst case situation. In
realistic use cases the overhead from unnecessary locking is difficult
to see.
Relative x11perf results showing the amount of regression for worst
case and some common operations are shown below.
unlocked/locked Operation
----------------- ------------------------------------
1.001 500-pixel line
1.000 Copy 500x500 from window to window
2.609 X protocol NoOperation
Signed-off-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
Reviewed-by: Erkki Seppälä <erkki.seppala at vincit.fi>
---
configure.ac | 37 +++++++++++++++++++++++++++++++++++++
src/Makefile.am | 8 ++++++++
src/locking.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
3 files changed, 95 insertions(+), 5 deletions(-)
diff --git a/configure.ac b/configure.ac
index a69735b..46152d1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -270,6 +270,38 @@ AC_ARG_ENABLE(xthreads,
[Disable Xlib support for Multithreading]),
[xthreads=$enableval],[xthreads=yes])
+# Determine whether the user wants to check for unsafe usage of Xlib
+# from multiple threads.
+AC_ARG_ENABLE(
+ checked_locks,
+ AS_HELP_STRING(
+ [--enable-checked-locks],
+ [Check if multithreaded clients have missed calling XInitThreads]),
+ [checked_locks=$enableval],
+ [checked_locks=no])
+
+# Check if XInitThreads can be called by default on library
+# initialization. The following conditions must be true for that.
+#
+# - Option --enable-xthreads is given.
+# - Option --enable-checked-locks is given.
+# - Compiler is GCC.
+# - GCC supports contructors.
+if test "x$xthreads" = xyes && test "x$checked_locks" = xyes && test "x$GCC" = xyes ; then
+ # Set warnings as errors so that GCC fails if the attribute is not
+ # supported.
+ old_CFLAGS="$CFLAGS"
+ CFLAGS="-Werror"
+ # Check if GCC recognizes constructor attribute.
+ AC_COMPILE_IFELSE(
+ [ void __attribute__((constructor)) test(void) {}; ],
+ checked_locks=yes,
+ checked_locks=no)
+ # Restore flags.
+ CFLAGS="$old_CFLAGS"
+fi
+AM_CONDITIONAL(XTHREADS_CHECKED_LOCKS, [test "x$checked_locks" = xyes])
+
AC_CHECK_LIB(c, getpwuid_r, [mtsafeapi="yes"], [mtsafeapi="no"])
case x$xthreads in
@@ -279,6 +311,10 @@ xyes)
then
AC_DEFINE(XUSE_MTSAFE_API,1,[Whether libX11 needs to use MT safe API's])
fi
+ if test x$checked_locks = xyes
+ then
+ AC_DEFINE(XTHREADS_CHECKED_LOCKS,1,[Whether multithreaded clients have missed calling XInitThreads])
+ fi
;;
*)
;;
@@ -481,6 +517,7 @@ echo " Loadable xcursor library support: "$XLIB_LOADABLE_XCURSOR
echo " Threading support: "$xthreads
echo " Use Threads safe API: "$mtsafeapi
echo " Threads stubs in libX11: "$thrstubs
+echo " Missing XInitThreads calls are detected: "$checked_locks
echo " XCMS: "$XCMS
echo " Internationalization support: "$XLOCALE
echo " XF86BigFont support: "$XF86BIGFONT
diff --git a/src/Makefile.am b/src/Makefile.am
index 05c7663..dfc07c6 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -366,12 +366,20 @@ if XKB
USE_XKB_LIBS = $(XKB_LIBS)
endif
+if XTHREADS_CHECKED_LOCKS
+USE_XTHREAD_CFLAGS = $(XTHREAD_CFLAGS)
+USE_XTHREAD_LIBS = $(XTHREADLIB)
+endif XTHREADS_CHECKED_LOCKS
+
+AM_CFLAGS += $(USE_XTHREAD_CFLAGS)
+
libX11_la_LDFLAGS = -version-number 6:3:0 -no-undefined
libX11_la_LIBADD = \
$(USE_I18N_LIBS) \
$(USE_XCMS_LIBS) \
$(USE_XKB_LIBS) \
+ $(USE_XTHREAD_LIBS) \
$(X11_LIBS)
preprocess: $(patsubst %.c,%.ii,$(libX11_la_SOURCES))
diff --git a/src/locking.c b/src/locking.c
index 4f9a40f..aa310d6 100644
--- a/src/locking.c
+++ b/src/locking.c
@@ -47,7 +47,7 @@ in this Software without prior written authorization from The Open Group.
#include "Xprivate.h"
#include "locking.h"
-#ifdef XTHREADS_WARN
+#if defined(XTHREADS_WARN) || defined(XTHREADS_CHECKED_LOCKS)
#include <stdio.h> /* for warn/debug stuff */
#endif
@@ -99,12 +99,43 @@ static xthread_t _Xthread_self(void)
static LockInfoRec global_lock;
static LockInfoRec i18n_lock;
+#ifdef XTHREADS_CHECKED_LOCKS
+/** User hasn't called XInitThreads yet and we wan't to detect whether
+ * Xlib is used incorrectly from multiple threads. */
+static Bool check_locks = True;
+#endif /* XTHREADS_CHECKED_LOCKS */
+
+/**
+ * Normally this function will just lock the parameter mutex. However,
+ * the function will abort if all of the conditions listed below hold.
+ *
+ * 1. Xlib has been configured using --enable-checked-locks option.
+ * 2. XInitThreads hasn't been called by user yet. Note that it has
+ * been called by the library from XInitLib.
+ * 3. The lock given as parameter hasn't been acquired already.
+ */
+static int xmutex_lock_checked(xmutex_t lock)
+{
+#ifdef XTHREADS_CHECKED_LOCKS
+ if (!check_locks)
+#endif /* XTHREADS_CHECKED_LOCKS */
+ return xmutex_lock(lock);
+#ifdef XTHREADS_CHECKED_LOCKS
+ if (!xmutex_trylock(lock))
+ return 0;
+ printf("Your program has been terminated because it entered a critical section\n"
+ "in Xlib simultaneously from more than one thread. To avoid this, you\n"
+ "should call XInitThreads before doing any other Xlib calls.\n");
+ abort();
+#endif /* XTHREADS_CHECKED_LOCKS */
+}
+
static void _XLockMutex(
LockInfoPtr lip
XTHREADS_FILE_LINE_ARGS
)
{
- xmutex_lock(lip->lock);
+ xmutex_lock_checked(lip->lock);
}
static void _XUnlockMutex(
@@ -172,7 +203,7 @@ static void _XLockDisplayWarn(
#endif /* XTHREADS_DEBUG */
}
- xmutex_lock(dpy->lock->mutex);
+ xmutex_lock_checked(dpy->lock->mutex);
if (strcmp(file, "XlibInt.c") == 0) {
if (!xlibint_unlock)
@@ -456,7 +487,7 @@ static void _XLockDisplay(
#ifdef XTHREADS_WARN
_XLockDisplayWarn(dpy, file, line);
#else
- xmutex_lock(dpy->lock->mutex);
+ xmutex_lock_checked(dpy->lock->mutex);
#endif
if (dpy->lock->locking_level > 0)
_XDisplayLockWait(dpy);
@@ -477,7 +508,7 @@ static void _XInternalLockDisplay(
#ifdef XTHREADS_WARN
_XLockDisplayWarn(dpy, file, line);
#else
- xmutex_lock(dpy->lock->mutex);
+ xmutex_lock_checked(dpy->lock->mutex);
#endif
if (!wskip && dpy->lock->locking_level > 0)
_XDisplayLockWait(dpy);
@@ -570,7 +601,13 @@ xthread_t (*_x11_thr_self)() = __x11_thr_self;
Status XInitThreads(void)
{
if (_Xglobal_lock)
+ {
+#ifdef XTHREADS_CHECKED_LOCKS
+ check_locks = False;
+#endif /* XTHREADS_CHECKED_LOCKS */
return 1;
+ }
+
#ifdef __UNIXWARE__
else {
void *dl_handle = dlopen(NULL, RTLD_LAZY);
@@ -615,6 +652,14 @@ Status XInitThreads(void)
return 1;
}
+#ifdef XTHREADS_CHECKED_LOCKS
+/** Initializes threads by default when Xlib is loaded. */
+static void __attribute__((constructor)) XInitLib(void)
+{
+ if (!XInitThreads())
+ abort();
+}
+#endif /* XTHREADS_CHECKED_LOCKS */
#else /* XTHREADS */
Status XInitThreads(void)
{
--
1.6.3.3
More information about the xorg-devel
mailing list