Re: gmodule mutex deadlock problem

[Tim Janik <timj gtk org>]

On 29 Dec 2000, Martin Baulig wrote:

> Tim Janik <timj gtk org> writes:
> 
> > On 28 Dec 2000, Martin Baulig wrote:
> > 
> > > there's a bug in gmodule which causes a deadlock when
> > > threads are enabled:
> > > 
> > > The problem is that g_module_open() calls g_module_symbol()
> > > while holding a lock on the g_module_global_lock._
> > 
> > eh? that shouldn't be the problem as g_module_global_lock is a
> > recursive mutex (though maybe there're prblems with recursive
> > mutexes, dunno). could you give a more detailed description
> > of how to trigger this (and please Cc: Sebastian on that
> > as he's our threading guru ;).
> 
> Ok, after some debugging I finally found the problem.

cool, thanks.

> In configure.in in line 1114 we have:
> 
> ---
> glib_cv_sizeof_system_thread=`expr $glib_cv_sizeof_system_thread + $ac_cv_sizeof_long`
> ---
> 
> This is wrong, you cannot modify a cache variable in this way. Next time
> configure is run without deleting config.cache (for instance, from
> config.status --recheck), glib_cv_sizeof_system_thread already has the
> incremented value and gets incremented another time.
> 
> Btw. there's also another bug with this, it doesn't even work the first time
> configure is run; I end up having GLIB_SIZEOF_SYSTEM_THREAD == 4, but
> glib_cv_sizeof_system_thread == 8 (because it is already AC_DEFINE'd before
> this expression).

ok, then, can you confirm that the following patch fixes things for you?

Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/glib/ChangeLog,v
retrieving revision 1.575
diff -u -u -r1.575 ChangeLog
--- ChangeLog	2000/12/29 02:16:49	1.575
+++ ChangeLog	2000/12/29 14:48:55
@@ -1,3 +1,10 @@
+Fri Dec 29 14:53:18 2000  Tim Janik  <timj gtk org>
+
+	* configure.in: we can't grow _cv_ variables by using a backticked
+	expr that refers back to the variable (glib_cv_sizeof_system_thread for
+	G_THREAD_USE_PID_SURROGATE), that'd keep the variable growing every time
+	it's evaluated. quantum states, anyone?)
+
 Thu Dec 28 10:21:46 2000  Tim Janik  <timj gtk org>
 
 	* gmem.[hc]: got rid of outdated dmalloc support. provide g_try_malloc()
Index: acconfig.h
===================================================================
RCS file: /cvs/gnome/glib/acconfig.h,v
retrieving revision 1.32
diff -u -u -r1.32 acconfig.h
--- acconfig.h	2000/12/29 02:16:49	1.32
+++ acconfig.h	2000/12/29 14:48:55
@@ -45,3 +45,4 @@
 
 #undef USE_LIBICONV
 #undef SANE_MALLOC_PROTOS
+#undef GLIB_SIZEOF_SYSTEM_THREAD
Index: configure.in
===================================================================
RCS file: /cvs/gnome/glib/configure.in,v
retrieving revision 1.165
diff -u -u -r1.165 configure.in
--- configure.in	2000/12/29 02:16:49	1.165
+++ configure.in	2000/12/29 14:48:57
@@ -1037,9 +1037,8 @@
 	if test x"$have_threads" = xposix; then
 		glib_save_CPPFLAGS="$CPPFLAGS"
 		CPPFLAGS="$CPPFLAGS $GTHREAD_COMPILE_IMPL_DEFINES"
-		GLIB_SIZEOF([#include <pthread.h>],
-			pthread_t,
-			system_thread)
+		dnl we might grow sizeof(pthread_t) later on, so use a dummy name here
+		GLIB_SIZEOF([#include <pthread.h>], pthread_t, core_system_thread)
 		# This is not AC_CHECK_FUNC to also work with function
 		# name mangling in header files.
 		AC_MSG_CHECKING(for pthread_attr_setstacksize)
@@ -1093,6 +1092,7 @@
 		CPPFLAGS="$glib_save_CPPFLAGS"
 
 		AC_MSG_CHECKING(whether to use the PID niceness surrogate for thread priorities)
+	        glib_system_thread_size_growth=0
 		AC_TRY_RUN([#include <pthread.h> 
 			#include <sys/types.h>
 			#include <unistd.h>
@@ -1108,16 +1108,15 @@
 				$posix_priority_min != $posix_priority_max);
 			}],
 			[AC_MSG_RESULT(yes),
-			 AC_DEFINE(G_THREAD_USE_PID_SURROGATE, 1,
-				[whether to use the PID niceness surrogate for thread priorities])
-
-			 glib_cv_sizeof_system_thread=`expr $glib_cv_sizeof_system_thread + $ac_cv_sizeof_long`],
+			  AC_DEFINE(G_THREAD_USE_PID_SURROGATE, 1, [whether to use the PID niceness surrogate for thread priorities])
+                          glib_system_thread_size_growth=$ac_cv_sizeof_long
+                        ],
 			[AC_MSG_RESULT(no)])
-	   
+                dnl make sure both glib_cv_sizeof_system_thread and GLIB_SIZEOF_SYSTEM_THREAD are setup
+                glib_cv_sizeof_system_thread=`expr $glib_cv_sizeof_core_system_thread + $glib_system_thread_size_growth`
+                AC_DEFINE_UNQUOTED(GLIB_SIZEOF_SYSTEM_THREAD, $glib_cv_sizeof_system_thread)
 	else # solaris threads
-		GLIB_SIZEOF([#include <thread.h>],
-			thread_t,
-			system_thread)
+		GLIB_SIZEOF([#include <thread.h>], thread_t, system_thread)
 	fi
 
 	LIBS="$glib_save_LIBS"
@@ -1738,7 +1737,7 @@
 
 g_mutex_has_default="$mutex_has_default"
 g_mutex_sizeof="$glib_cv_sizeof_gmutex"
-g_system_thread_sizeof="$glib_cv_sizeof_system_thread"
+g_system_thread_sizeof="$glib_cv_sizeof_system_thread"
 g_mutex_contents="$glib_cv_byte_contents_gmutex"
 
 g_module_suffix="$glib_gmodule_suffix"


> 
> -- 
> Martin Baulig
> martin gnome org (private)
> baulig suse de (work)
> 
> 

---
ciaoTJ