>Number:         37654
>Category:       lib
>Synopsis:       libc's atexit_mutex should be fully recursive
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Dec 31 21:20:00 +0000 2007
>Originator:     Sverre Froyen
>Release:        NetBSD 4.99.45
>Organization:
	Viewmark
>Environment:
System: NetBSD abbor.fesk.com 4.99.45 NetBSD 4.99.45 (ABBOR) #56: Tue Dec 25 11:38:12 MST 2007 toor@abbor.fesk.com:/usr/src/objdir/sys/arch/i386/compile.i386/ABBOR i386
Architecture: i386
Machine: i386
>Description:
The QT4 qdbus command is a QT C++ utility that when destroying objects
after the call to exit(3) causes the method __cxa_atexit to be called
from within __cxa_finalize (__cxa_atexit and __cxa_finalize are libc
methods).  Since both __cxa_atexit and __cxa_finalize acquire atexit_mutex,
and since the mutex is not recursive this causes qdbus to hang.
Here's a backtrace from when qdbus is looping in pthread__mutex_spin:

(gdb) bt
#0  pthread__mutex_spin (ptm=0xbb86dba0, owner=<value optimized out>)
    at /usr/src/lib/libpthread/pthread_mutex2.c:206
#1  0xbbb56a63 in pthread__mutex_lock_slow (ptm=0xbb86dba0)
    at /usr/src/lib/libpthread/pthread_mutex2.c:248
#2  0xbb83fb8c in __cxa_atexit () from /usr/lib/libc.so.12
#3  0xbba8f5ee in guardHash () at kernel/qobject.cpp:420
#4  0xbba92896 in QObjectPrivate::clearGuards (object=0xbb602430)
    at kernel/qobject.cpp:487
#5  0xbba94be5 in ~QObject (this=0xbb602430) at kernel/qobject.cpp:799
#6  0xbb9ec75b in ~QThread (this=0xbb602430) at thread/qthread.cpp:354
#7  0xbba70485 in ~QProcessManager (this=0xbb602430)
    at io/qprocess_unix.cpp:270
#8  0xbba6f0d0 in __tcf_0 ()
    at ../../include/QtCore/../../src/corelib/global/qglobal.h:1434
#9  0xbb83fab6 in __cxa_finalize () from /usr/lib/libc.so.12
#10 0xbb83f962 in exit () from /usr/lib/libc.so.12
#11 0x08051356 in main (argc=-1080033280, argv=0x11110001) at qdbus.cpp:391

I gave this problem priority high because it is likely to affect the soon
to be released KDE4.

>How-To-Repeat:
Build and install x11/qt4-qdbus from pkgsrc and run qdbus.

>Fix:
Apply the following patch which makes atexit_mutex recursive:

Index: lib/libc/stdlib/atexit.c
===================================================================
RCS file: /cvsroot/src/lib/libc/stdlib/atexit.c,v
retrieving revision 1.19
diff -u -r1.19 atexit.c
--- lib/libc/stdlib/atexit.c	8 Aug 2007 01:05:34 -0000	1.19
+++ lib/libc/stdlib/atexit.c	31 Dec 2007 19:43:39 -0000
@@ -81,7 +81,8 @@
 
 #ifdef _REENTRANT
 /* ..and a mutex to protect it all. */
-static mutex_t atexit_mutex = MUTEX_INITIALIZER;
+static mutex_t atexit_mutex;
+uint atexit_mutex_ready = 0;
 #endif /* _REENTRANT */
 
 /*
@@ -119,6 +120,20 @@
 }
 
 /*
+ * Initialize atexit_mutex with the PTHREAD_MUTEX_RECURSIVE attribute.
+ * Note that __cxa_finalize may generate calls to __cxa_atexit.
+ */
+static void
+atexit_mutex_init(void)
+{
+	mutexattr_t atexit_mutex_attr;
+	mutexattr_init(&atexit_mutex_attr);
+	mutexattr_settype(&atexit_mutex_attr, PTHREAD_MUTEX_RECURSIVE);
+	mutex_init(&atexit_mutex, &atexit_mutex_attr);
+	atexit_mutex_ready = 1;
+}
+
+/*
  * Register an atexit routine.  This is suitable either for a cxa_atexit
  * or normal atexit type handler.  The __cxa_atexit() name and arguments
  * are specified by the C++ ABI.  See:
@@ -132,6 +147,8 @@
 
 	_DIAGASSERT(func != NULL);
 
+	if (! atexit_mutex_ready)
+		atexit_mutex_init();
 	mutex_lock(&atexit_mutex);
 
 	ah = atexit_handler_alloc(dso);
@@ -162,23 +179,14 @@
 void
 __cxa_finalize(void *dso)
 {
-	static thr_t owner;
 	static u_int call_depth;
 	struct atexit_handler *ah, *dead_handlers = NULL, **prevp;
 	void (*cxa_func)(void *);
 	void (*atexit_func)(void);
 
-	/*
-	 * We implement our own recursive mutex here because we need
-	 * to keep track of the call depth anyway, and it saves us
-	 * having to dynamically initialize the mutex.
-	 */
-	if (mutex_trylock(&atexit_mutex) == 0)
-		owner = thr_self();
-	else if (owner != thr_self()) {
-		mutex_lock(&atexit_mutex);
-		owner = thr_self();
-	}
+	if (! atexit_mutex_ready)
+		atexit_mutex_init();
+	mutex_lock(&atexit_mutex);
 
 	call_depth++;
 
@@ -224,13 +232,13 @@
 			prevp = &ah->ah_next;
 	}
 
+	mutex_unlock(&atexit_mutex);
+
 	call_depth--;
 
 	if (call_depth > 0)
 		return;
 
-	mutex_unlock(&atexit_mutex);
-
 	/*
 	 * Now free any dead handlers.  Do this even if we're about to
 	 * exit, in case a leak-detecting malloc is being used.