Re: next round: glib thread safety proposal

[Sebastian Wilhelmi <wilhelmi ira uka de>]

Hi, Owen

> I've had a chance to take a look at this now, and my general
> impression is while the solution as implemented with weak
> symbols and __attribute__ ((constructor)) is quite clever
> and interesting, once you consider making it portable beyond
> gcc and ELF, the complexity it introduces is a bit
> prohibitive.

Yes, especially the ((construtor)) thing is questionable.

> My inclination then is to go with a simpler solution - the
> default implementation (in terms of pthreads, or other
> threads) is put into a tiny library, -lgthread, and
> programs wanting to use the default implementation call
> g_thread_init() as the first thing in their program.
> 
>  glib-config --libs gthread
> 
> then returns '-lglib -lgthread -lpthread', to allow
> configure scripts to get the correct libraries
> easily.
> 
> This solution is not incompatible with eventually doing
> clever tricks with weak symbols to avoid the need for
> another library, but I think it provides a simpler
> system at a fairly small additional cost. Are there
> any problems with this approach that I am missing?

I think, its more error prone on most of the systems, that do have the gcc
and ELF (i.e. weak symbols), because here some people might miss to
initialize the threads and such experience 'bugs'. So it seems to become a
new FAQ (a'la 'I want to connect a signal to my C++ class member function'
;-), but I admit, the automatic initialization could be added later
without much hassle also for the g_thread_init() approach.

> > So do something like:
> >
> > G_MUTEX_STANDARD_IMPLEMENTATION;
> >
> > int
> > main()
> > {
> >   /* working thread safe on platforms with and without weak symbols */
> > }
> 
> I don't see how a single macro could do both of these (especially
> in the absence of weak symbols). The user would have to call some
> function inside there main(), I would think, which brings us
> back to g_thread_init().

Yes, that was my mistake. I though of an initialization of the function
table, but that would only work, if defined at the same time, which isn't
possible, as it already is defined  external (in the lib). So that would
indeed suggest a solution with g_thread_init. and here it is: a new
proposal: there are three main changes from the old one:

- it was a bad idea to use the duration as a relative time. we have to use
an absolute ending time for the wait. this however doesn't fit to well
into the current setup. I added a type GAbsTime, which basically is struct
tm, but as this type is not to be found on win32, I added a new type. I
actually do also think that to use this would be better for the new glib
mainloop too. we could then have some convienience functions to manipulate
them, i.e. get the current time, add n microseconds, subtract another
AbsTime and so on. 

- I added GStaticMutex, that can be used after being initialized
statically. Much more convenient, than having to do dynamic initialization
on your own. This however builds on GMutex, and as such is slower (but
just one non-zero-test after first time use) 

- we do need the thread specific data function. because the
allocating_for_mem_chunk counter in gmem.c must be thread specific. I have
however not done anything about it, because we could work around that
particular problem and that might be better anyway. (I havn't looked yet,
if there are other cases, where glib would need thread specific data, but
it might be handy to have, could be added later though, it might be best
just to get the current aspect right before that.)

so attached as usual a patch and a tar (BTW: would it be simpler for you
or anybody to not post it, but put it on the ftp-server???)

I already did a decent configure.in extension and I moved most of the
stuff to a new directory gthread/, because the whole thing might be
extended to support threads someday, and as it is an extra library, I
thought this would be the right thing. I also did an extra support for
solaris mutexes as posix mutexes are simulated using solaris mutexes on
solaris and thus solaris mutexes are faster than the posix ones.

Bye,
Sebastian
-- 
+--------------============####################============--------------+
| Sebastian Wilhelmi, Institute for Computer Design and Fault Tolerance, |
| University of Karlsruhe;  Building 20.20, Room 263,  D-76128 Karlsruhe |
| mail: wilhelmi@ira.uka.de;  fax: +49 721 370455;  fon: +49 721 6084353 |
+-----------------> http://goethe.ira.uka.de/~wilhelmi <-----------------+
? gthread.c
? gmutex.c
? gthread
? libgthread.la
Index: Makefile.am
===================================================================
RCS file: /cvs/gnome/glib/Makefile.am,v
retrieving revision 1.14
diff -u -r1.14 Makefile.am
--- Makefile.am	1998/11/12 04:28:49	1.14
+++ Makefile.am	1998/11/26 14:59:11
@@ -1,7 +1,7 @@
 ## Process this file with automake to produce Makefile.in
 
 # build . first, then SUBDIRS
-SUBDIRS = gmodule docs
+SUBDIRS = gmodule gthread docs 
 all-recursive-am: all-am
 # alpha `automake' supports this better
 #SUBDIRS = . gmodule docs
@@ -46,7 +46,8 @@
 		gstring.c	\
 		gstrfuncs.c	\
 		gscanner.c	\
-		gutils.c
+		gutils.c	\
+		gmutex.c
 
 include_HEADERS = \
 		glib.h
Index: acconfig.h
===================================================================
RCS file: /cvs/gnome/glib/acconfig.h,v
retrieving revision 1.5
diff -u -r1.5 acconfig.h
--- acconfig.h	1998/10/27 04:11:10	1.5
+++ acconfig.h	1998/11/26 14:59:11
@@ -82,6 +82,8 @@
 #undef WIN32
 #undef NATIVE_WIN32
 
+#undef G_THREAD_SOURCE
+
 /* #undef PACKAGE */
 /* #undef VERSION */
 
Index: configure.in
===================================================================
RCS file: /cvs/gnome/glib/configure.in,v
retrieving revision 1.27
diff -u -r1.27 configure.in
--- configure.in	1998/11/23 01:37:29	1.27
+++ configure.in	1998/11/26 14:59:13
@@ -506,6 +506,134 @@
 AC_SUBST(G_MODULE_HAVE_DLERROR)
 AC_SUBST(G_MODULE_NEED_USCORE)
 
+
+dnl ***********************
+dnl *** g_thread checks ***
+dnl ***********************
+
+AC_ARG_ENABLE(thread, [  --enable-thread=[none/posix/solaris] specify a thread implementation to use.],,)
+
+dnl error and warning message
+dnl *************************
+
+THREAD_NO_IMPLEMENTATION="You do not have any known thread system on your
+                computer. glib will not be thread safe on your computer."
+
+THREAD_UNKNOWN_COMPILER="Your compiler is not known, so I cannot
+                determine the necessary compiler options to compile programs 
+                which are using threads. Please provide such information."
+
+FLAG_DOES_NOT_WORK="I can't find the MACRO, that enables thread safety on your
+                platform (normaly it's "_REENTRANT"). I'll not use any flag on
+                compilation now, but then your programs might not work. 
+                Please provide information on how it is done on your system."
+
+LIBS_NOT_FOUND_1="I can't find the libraries for the thread implementation
+		"
+
+LIBS_NOT_FOUND_2=". Please choose another thread implementation or 
+		provide informationon your thread implementation."
+
+dnl determination of thread implementation
+dnl ***************************************
+
+if test x"$enable_thread" = x; then
+        case $host in
+                *-*-solaris*)
+                        AC_CHECK_LIB(thread,cond_init,enable_thread=solaris)
+                        ;;
+        esac
+        if test x"$enable_thread" = x; then
+                AC_CHECK_LIB(pthread,pthread_cond_init,enable_thread=posix)
+     	        AC_CHECK_LIB(pthreads,pthread_attr_init,enable_thread=posix)
+	fi
+fi
+
+AC_MSG_CHECKING(for thread implementation)
+
+if test x"$enable_thread" = x; then
+        enable_thread=none
+        AC_MSG_WARN($THREAD_NO_IMPLEMENTATION)
+fi
+
+AC_MSG_RESULT($enable_thread)
+
+dnl determination of G_THREAD_LIBS
+dnl ******************************
+
+G_THREAD_LIBS=
+
+case $enable_thread in
+        posix)
+		G_THREAD_LIBS=error
+		AC_CHECK_LIB(pthreads,pthread_cond_init,
+			     G_THREAD_LIBS="-lpthreads") 
+                AC_CHECK_LIB(pthread,pthread_cond_init,
+			     G_THREAD_LIBS="-lpthread")     	        
+
+                ;;
+        solaris)
+		G_THREAD_LIBS=error
+                AC_CHECK_LIB(thread,cond_init,G_THREAD_LIBS="-lthread")
+                ;;
+        none)
+                ;;
+        *)
+		G_THREAD_LIBS=error
+                ;;
+esac
+
+if test "x$G_THREAD_LIBS" = xerror; then
+        AC_MSG_ERROR($LIBS_NOT_FOUND_1$enable_thread$LIBS_NOT_FOUND_2)
+fi
+
+AC_MSG_CHECKING(necessary linker options)
+AC_MSG_RESULT($G_THREAD_LIBS)
+
+dnl determination of G_THREAD_CFLAGS
+dnl ********************************
+
+if test x"$enable_thread" != xnone; then
+	G_THREAD_CFLAGS="-D_REENTRANT" # good default
+
+	case $host in
+ 	       -aix*)
+			# can somebody confirm this -D_THREAD_SAFE ???
+			G_THREAD_CFLAGS="$G_THREAD_CFLAGS -D_THREAD_SAFE"
+			if test x"$GCC" = xyes; then 
+                	        G_THREAD_CFLAGS="$G_THREAD_CFLAGS -mthreads"
+			fi
+			;;
+	esac
+
+	old_CPPFLAGS=$CPPFLAGS
+	CPPFLAGS="$CPPFLAGS $G_THREAD_CFLAGS"
+	AC_EGREP_HEADER([[^a-zA-Z_]ctime_r[^a-zA-Z_]], time.h, , 
+        	G_THREAD_CFLAGS=
+        	AC_MSG_WARN($FLAG_DOES_NOT_WORK))
+	CPPFLAGS=$old_CPPFLAGS
+                
+	if test x"$GCC" = xyes; then 
+        	G_THREAD_CFLAGS="$G_THREAD_CFLAGS -fstack-check"
+	else
+        	AC_MSG_WARN($THREAD_UNKNOWN_COMPILER)
+	fi
+
+	AC_MSG_CHECKING(necessary compiler options)
+
+	AC_MSG_RESULT($G_THREAD_CFLAGS)
+else
+        G_THREAD_CFLAGS=
+fi
+
+AC_DEFINE_UNQUOTED(G_THREAD_SOURCE,"gthread-$enable_thread.c")
+AC_SUBST(G_THREAD_CFLAGS)
+AC_SUBST(G_THREAD_LIBS)
+
+dnl ******************************
+dnl *** output the whole stuff ***
+dnl ******************************
+
 AC_OUTPUT_COMMANDS([
 
 ## Generate `glibconfig.h' in two cases
@@ -835,6 +963,7 @@
 glib-config
 gmodule/gmoduleconf.h
 gmodule/Makefile
+gthread/Makefile
 docs/Makefile
 ],[case "$CONFIG_FILES" in
 *glib-config*)chmod +x glib-config;;
Index: glib-config.in
===================================================================
RCS file: /cvs/gnome/glib/glib-config.in,v
retrieving revision 1.3
diff -u -r1.3 glib-config.in
--- glib-config.in	1998/09/15 19:08:07	1.3
+++ glib-config.in	1998/11/26 14:59:14
@@ -17,6 +17,7 @@
 Libraries:
 	glib
 	gmodule
+	gthread
 EOF
 	exit $1
 }
@@ -69,6 +70,9 @@
     gmodule)
       lib_gmodule=yes
       ;;
+    gthread)
+      lib_gthread=yes
+      ;;
     *)
       usage 1 1>&2
       ;;
@@ -83,7 +87,11 @@
 	echo $exec_prefix
 fi
 if test "$echo_cflags" = "yes"; then
-	echo -I@libdir@/glib/include $includes
+	cflags=""
+	if test "$lib_gthread" = "yes"; then
+		cflags="$cflags @G_THREAD_CFLAGS@"
+	fi
+	echo -I@libdir@/glib/include $includes $cflags
 fi
 if test "$echo_libs" = "yes"; then
 	libs=""
@@ -92,6 +100,9 @@
 	fi
 	if test "$lib_gmodule" = "yes"; then
 		libs="@G_MODULE_LDFLAGS@ -lgmodule $libs @G_MODULE_LIBS@"
+	fi
+	if test "$lib_gthread" = "yes"; then
+		libs="-lgthread $libs @G_THREAD_LIBS@"
 	fi
-	echo "-L@libdir@ $libs"
+	echo -L@libdir@ $libs
 fi
Index: glib.h
===================================================================
RCS file: /cvs/gnome/glib/glib.h,v
retrieving revision 1.79
diff -u -r1.79 glib.h
--- glib.h	1998/11/25 03:02:48	1.79
+++ glib.h	1998/11/26 14:59:15
@@ -2185,6 +2185,70 @@
 
 #endif /* NATIVE_WIN32 */
 
+/* Time since 1st Jan 1970, 00:00 UTC */
+typedef struct _GAbsTime GAbsTime;
+struct _GAbsTime
+{
+  gulong seconds;
+  gulong microseconds;
+};
+
+typedef struct _GMutex GMutex;
+typedef GMutex* GStaticMutex;
+typedef struct _GCond GCond;
+
+#define G_STATIC_MUTEX_INIT NULL;
+
+typedef struct _GMutexFunctions GMutexFunctions;
+struct _GMutexFunctions
+{
+  GMutex*  (*mutex_new)       ();
+  void     (*mutex_lock)      (GMutex* mutex);
+  gboolean (*mutex_try_lock)  (GMutex* mutex);
+  void     (*mutex_unlock)    (GMutex* mutex);
+  void     (*mutex_free)      (GMutex* mutex);
+  GCond*   (*cond_new)        ();
+  void     (*cond_signal)     (GCond* cond);
+  void     (*cond_broadcast)  (GCond* cond);
+  void     (*cond_wait)       (GCond* cond, GMutex* mutex);
+  gboolean (*cond_timed_wait) (GCond* cond, GMutex* mutex, 
+			       GAbsTime* abs_time);
+  void     (*cond_free)       (GCond* cond);
+};
+
+extern GMutexFunctions g_mutex_functions_for_glib_use;
+extern gboolean g_mutex_supported;
+extern gboolean g_cond_supported;
+
+void g_thread_init(GMutexFunctions* init);
+GStaticMutex g_static_mutex_init(GStaticMutex* mutex);
+
+#define G_USE_MUTEX_FUNC(name,fail,arg) \
+  (g_mutex_supported ? (*g_mutex_functions_for_glib_use.name)arg : (fail))
+#define G_USE_COND_FUNC(name,fail,arg) \
+  (g_cond_supported ? (*g_mutex_functions_for_glib_use.name)arg : (fail))
+
+#define g_mutex_new() G_USE_MUTEX_FUNC(mutex_new,NULL,())
+#define g_mutex_lock(mutex) G_USE_MUTEX_FUNC(mutex_lock,0,(mutex))
+#define g_mutex_try_lock(mutex) G_USE_MUTEX_FUNC(mutex_try_lock,TRUE,(mutex))
+#define g_mutex_unlock(mutex) G_USE_MUTEX_FUNC(mutex_unlock,0,(mutex))
+#define g_mutex_free(mutex) G_USE_MUTEX_FUNC(mutex_free,0,(mutex))
+#define g_cond_new() G_USE_COND_FUNC(cond_new,NULL,())
+#define g_cond_signal(cond) G_USE_COND_FUNC(cond_signal,0,(cond))
+#define g_cond_broadcast(cond) G_USE_COND_FUNC(cond_broadcast,0,(cond))
+#define g_cond_wait(cond,mutex) G_USE_COND_FUNC(cond_wait,0,(cond,mutex))
+#define g_cond_timed_wait(cond,mutex,abs_time) \
+  G_USE_COND_FUNC(cond_timed_wait,TRUE,(cond,mutex,abs_time))
+#define g_cond_free(cond) G_USE_COND_FUNC(cond_free,0,(cond))
+
+#define g_static_mutex_get_mutex(mutex) \
+  ( (!(mutex) && g_mutex_supported) ? g_static_mutex_init(&(mutex)) : (mutex) )
+#define g_static_mutex_lock(mutex) \
+  g_mutex_lock( g_static_mutex_get_mutex(mutex) )
+#define g_static_mutex_try_lock(mutex) \
+  g_mutex_try_lock( g_static_mutex_get_mutex(mutex) )
+#define g_static_mutex_unlock(mutex) \
+  ((mutex)? g_mutex_unlock( mutex ): 0)
 
 #ifdef __cplusplus
 }
Index: testglib.c
===================================================================
RCS file: /cvs/gnome/glib/testglib.c,v
retrieving revision 1.16
diff -u -r1.16 testglib.c
--- testglib.c	1998/11/13 20:50:41	1.16
+++ testglib.c	1998/11/26 15:00:54
@@ -870,5 +870,22 @@
 #endif
   g_print ("ok\n");
 
+  {
+    GMutex* mutex = g_mutex_new();
+    GCond* cond = g_cond_new();
+    GStaticMutex static_mutex = G_STATIC_MUTEX_INIT;
+
+    g_mutex_lock(mutex);
+    g_mutex_unlock(mutex);
+
+    g_static_mutex_lock(static_mutex);
+    g_static_mutex_unlock(static_mutex);
+    
+    g_cond_signal(cond);
+    g_cond_broadcast(cond);
+
+    g_cond_free(cond);
+    g_mutex_free(mutex);
+  }
   return 0;
 }

gthread.tar