errorcheck mutexes

[Sebastian Wilhelmi <wilhelmi ira uka de>]

Hi,

I have developed a nice new toy, which I'll check in next week, when no
objections emerge.

You can now (by defining G_ERRORCHECK_MUTEXES) enable error check mutexes. 
These mutexes report, whenever you try to relock an alredy locked mutex and if
you try to unlock an already unlocked mutex.
First you have to enable it for every *.c file, that should be able to report
where the errors occur (There all calls to g_mutex_... are instrumented with
name of the mutex and location) and you have to enable it for the file, where
g_thread_init is called. But you don't need to enable it for every .c-file in
a given binary, even if mutexes are passed between those files and it still
works (only without location info then).

As an example I have (just for the purpose of demonstrating it) changed the
thread-test program to contain a double lock. Just make it without error info.
Then it just hangs. Or do 'make CFLAGS+="-DG_ERRORCHECK_MUTEXES"' and run
again.

The patch is attached.
-- 
Sebastian Wilhelmi                   |            här ovanför alla molnen
mailto:wilhelmi ira uka de           |     är himmlen så förunderligt blå
http://goethe.ira.uka.de/~wilhelmi   |

Index: configure.in
===================================================================
RCS file: /cvs/gnome/glib/configure.in,v
retrieving revision 1.143
diff -u -b -B -r1.143 configure.in
--- configure.in	2000/09/21 12:49:10	1.143
+++ configure.in	2000/09/22 13:38:16
@@ -1304,16 +1304,19 @@
 struct _GStaticMutex
 {
   struct _GMutex *runtime_mutex;
+  struct {
   union {
     char   pad[$g_mutex_sizeof];
     double dummy_double;
     void  *dummy_pointer;
     long   dummy_long;
-  } aligned_pad_u;
+    } mutex;
+    void *debug_info;
+  } static_mutex;
 };
-#define	G_STATIC_MUTEX_INIT	{ NULL, { { $g_mutex_contents} } }
+#define	G_STATIC_MUTEX_INIT	{ NULL, { { { $g_mutex_contents} }, NULL } }
 #define	g_static_mutex_get_mutex(mutex) \
-  (g_thread_use_default_impl ? ((GMutex*) &((mutex)->aligned_pad_u)) : \
+  (g_thread_use_default_impl ? ((GMutex*) &((mutex)->static_mutex)) : \
    g_static_mutex_get_mutex_impl (&((mutex)->runtime_mutex)))
 _______EOF
 	else
Index: glib.h
===================================================================
RCS file: /cvs/gnome/glib/glib.h,v
retrieving revision 1.199
diff -u -b -B -r1.199 glib.h
--- glib.h	2000/09/19 14:30:35	1.199
+++ glib.h	2000/09/22 13:38:17
@@ -3140,6 +3140,22 @@
  */
 void	g_thread_init	(GThreadFunctions	*vtable);
 
+/* Errorcheck mutexes. If you define G_ERRORCHECK_MUTEXES, then all
+ * mutexes will check for re-locking and re-unlocking */
+
+/* Initialize thread system with errorcheck mutexes. vtable must be
+ * NULL.Do not call directly. Use #define G_ERRORCHECK_MUTEXES
+ * instead.  
+ */
+void    g_thread_init_with_errorcheck_mutexes (GThreadFunctions* vtable);
+
+/* A random number to recognize debug calls to g_mutex_... */
+#define G_MUTEX_DEBUG_MAGIC 0xf8e18ad7
+
+#ifdef G_ERRORCHECK_MUTEXES
+#define g_thread_init(vtable) g_thread_init_with_errorcheck_mutexes (vtable) 
+#endif
+
 /* internal function for fallback static mutex implementation */
 GMutex*	g_static_mutex_get_mutex_impl	(GMutex	**mutex);
 
@@ -3148,14 +3164,25 @@
     (*g_thread_functions_for_glib_use . name) arglist
 #define G_THREAD_CF(name, fail, arg) \
     (g_thread_supported () ? G_THREAD_UF (name, arg) : (fail))
-/* keep in mind, all those mutexes and static mutexes are not 
- * recursive in general, don't rely on that
- */
+#define G_THREAD_ECF(name, fail, mutex, mutexname, type)		\
+    (g_thread_supported () ? 						\
+     ((type(*)(GMutex*, gulong, gchar*, gchar*))			\
+      (*g_thread_functions_for_glib_use . name)) 			\
+     (mutex, G_MUTEX_DEBUG_MAGIC, G_STRINGIFY (mutexname),  G_STRLOC) : (fail))
 #define	g_thread_supported()	(g_threads_got_initialized)
 #define g_mutex_new()            G_THREAD_UF (mutex_new,      ())
+#ifndef G_ERRORCHECK_MUTEXES
 #define g_mutex_lock(mutex)      G_THREAD_CF (mutex_lock,     (void)0, (mutex))
 #define g_mutex_trylock(mutex)   G_THREAD_CF (mutex_trylock,  TRUE,    (mutex))
 #define g_mutex_unlock(mutex)    G_THREAD_CF (mutex_unlock,   (void)0, (mutex))
+#else /* G_ERRORCHECK_MUTEXES */
+#define g_mutex_lock(mutex)						\
+  G_THREAD_ECF (mutex_lock,    (void)0, mutex, mutex, void)
+#define g_mutex_trylock(mutex)   					\
+  G_THREAD_ECF (mutex_trylock, TRUE,    mutex, mutex, gboolean)
+#define g_mutex_unlock(mutex)						\
+  G_THREAD_ECF (mutex_unlock,  (void)0, mutex, mutex, void)
+#endif /* G_ERRORCHECK_MUTEXES */
 #define g_mutex_free(mutex)      G_THREAD_CF (mutex_free,     (void)0, (mutex))
 #define g_cond_new()             G_THREAD_UF (cond_new,       ())
 #define g_cond_signal(cond)      G_THREAD_CF (cond_signal,    (void)0, (cond))
@@ -3195,12 +3222,24 @@
  * much easier, than having to explicitly allocate the mutex before
  * use
  */
+#ifndef G_ERRORCHECK_MUTEXES
 #define g_static_mutex_lock(mutex) \
     g_mutex_lock (g_static_mutex_get_mutex (mutex))
 #define g_static_mutex_trylock(mutex) \
     g_mutex_trylock (g_static_mutex_get_mutex (mutex))
 #define g_static_mutex_unlock(mutex) \
     g_mutex_unlock (g_static_mutex_get_mutex (mutex)) 
+#else /* G_ERRORCHECK_MUTEXES */
+#define g_static_mutex_lock(mutex)					\
+  G_THREAD_ECF (mutex_lock,    (void)0, 				\
+		g_static_mutex_get_mutex (mutex), mutex, void)
+#define g_static_mutex_trylock(mutex)   				\
+  G_THREAD_ECF (mutex_trylock, TRUE,  					\
+		g_static_mutex_get_mutex (mutex), mutex, gboolean)
+#define g_static_mutex_unlock(mutex)					\
+  G_THREAD_ECF (mutex_unlock,  (void)0, 				\
+		g_static_mutex_get_mutex (mutex), mutex, void)
+#endif /* G_ERRORCHECK_MUTEXES */
 
 struct _GStaticPrivate
 {
Index: gthread/gthread-impl.c
===================================================================
RCS file: /cvs/gnome/glib/gthread/gthread-impl.c,v
retrieving revision 1.4
diff -u -b -B -r1.4 gthread-impl.c
--- gthread/gthread-impl.c	2000/07/26 11:02:01	1.4
+++ gthread/gthread-impl.c	2000/09/22 13:38:21
@@ -48,6 +48,153 @@
 void g_mem_init (void);
 void g_messages_init (void);
 
+#define G_MUTEX_DEBUG_INFO(mutex) (*((gpointer*)(((char*)mutex)+G_MUTEX_SIZE)))
+
+typedef struct _ErrorCheckInfo ErrorCheckInfo;
+struct _ErrorCheckInfo
+{
+  gchar *name;
+  gchar *location;
+  GThread *owner;
+};
+
+static GMutex *
+g_mutex_new_errorcheck_impl (void)
+{
+  GMutex *retval = g_thread_functions_for_glib_use_default.mutex_new ();
+  retval = g_realloc (retval, G_MUTEX_SIZE + sizeof (gpointer));
+  G_MUTEX_DEBUG_INFO (retval) = NULL;
+  return retval;
+}
+
+static inline void
+fill_info (ErrorCheckInfo *info,
+	   GThread *self,
+	   gulong magic, 
+	   gchar *name, 
+	   gchar *location)
+{
+  info->owner = self;
+  if (magic == G_MUTEX_DEBUG_MAGIC)
+    {
+      /* We are used with special instrumented calls, where name and
+       * location is provided as well, so use them */
+      info->name = name;
+      info->location = location;
+    }
+  else
+    info->name = info->location = "unknown";
+}
+
+static void
+g_mutex_lock_errorcheck_impl (GMutex *mutex, 
+			      gulong magic, 
+			      gchar *name, 
+			      gchar *location)
+{
+  ErrorCheckInfo *info;
+  GThread *self = g_thread_self ();
+  if (G_MUTEX_DEBUG_INFO (mutex) == NULL)
+    {
+      /* if the debug info is NULL, we have not yet locked that mutex,
+       * so we do it now */
+      g_thread_functions_for_glib_use_default.mutex_lock (mutex);
+      /* Now we have to check again, because anothe thread might mave
+       * locked the mutex at the same time, we did. This technique is
+       * not 100% save on systems without decent cache coherence,
+       * but we have no choice */ 
+      if (G_MUTEX_DEBUG_INFO (mutex) == NULL)
+	{
+	  info = G_MUTEX_DEBUG_INFO (mutex) = g_new0 (ErrorCheckInfo, 1);
+	}
+      g_thread_functions_for_glib_use_default.mutex_unlock (mutex);
+    }
+  
+  info = G_MUTEX_DEBUG_INFO (mutex);
+  if (info->owner == self)
+    g_error ("Trying to recursivly lock the mutex '%s' at '%s', "
+	     "previously locked by name '%s' at '%s'", 
+	     name, location, info->name, info->location);
+
+  g_thread_functions_for_glib_use_default.mutex_lock (mutex);
+
+  fill_info (info, self, magic, name, location);
+}
+
+static gboolean
+g_mutex_trylock_errorcheck_impl (GMutex *mutex, 
+				 gulong magic, 
+				 gchar *name, 
+				 gchar *location)
+{
+  ErrorCheckInfo *info = G_MUTEX_DEBUG_INFO (mutex);
+  GThread *self = g_thread_self ();
+  if (!info)
+    {
+      /* This mutex has not yet been used, so simply lock and return TRUE */
+      g_mutex_lock_errorcheck_impl (mutex, magic, name, location);
+      return TRUE;
+    }
+  if (info->owner == self)
+    g_error ("Trying to recursivly lock the mutex '%s' at '%s', "
+	     "previously locked by name '%s' at '%s'", 
+	     name, location, info->name, info->location);
+  
+  if (!g_thread_functions_for_glib_use_default.mutex_trylock (mutex))
+    return FALSE;
+
+  fill_info (info, self, magic, name, location);
+  return TRUE;
+}
+
+static void
+g_mutex_unlock_errorcheck_impl (GMutex *mutex, 
+				gulong magic, 
+				gchar *name, 
+				gchar *location)
+{
+  ErrorCheckInfo *info = G_MUTEX_DEBUG_INFO (mutex);
+  GThread *self = g_thread_self ();
+  if (magic != G_MUTEX_DEBUG_MAGIC)
+    name = location = "unknown";
+  if (!info || info->owner != self)
+    g_error ("Trying to unlock the unlocked mutex '%s' at '%s'",
+	     name, location);
+  info->owner = NULL;
+  info->name = info->location = NULL;
+  g_thread_functions_for_glib_use_default.mutex_unlock (mutex);
+}
+
+static void
+g_mutex_free_errorcheck_impl (GMutex *mutex)
+{
+  g_free (G_MUTEX_DEBUG_INFO (mutex));
+  g_thread_functions_for_glib_use_default.mutex_free (mutex);  
+}
+
+/* unshadow function declaration. See glib.h */
+#undef g_thread_init
+
+void 
+g_thread_init_with_errorcheck_mutexes (GThreadFunctions* init)
+{
+  GThreadFunctions errorcheck_functions;
+  if (init)
+    g_error ("Errorcheck mutexes can only be used for native " 
+	     "thread implementations. Sorry." );
+  errorcheck_functions = g_thread_functions_for_glib_use_default;
+  errorcheck_functions.mutex_new = g_mutex_new_errorcheck_impl;
+  errorcheck_functions.mutex_lock = 
+    (void (*)(GMutex *)) g_mutex_lock_errorcheck_impl;
+  errorcheck_functions.mutex_trylock = 
+    (gboolean (*)(GMutex *)) g_mutex_trylock_errorcheck_impl;
+  errorcheck_functions.mutex_unlock = 
+    (void (*)(GMutex *)) g_mutex_unlock_errorcheck_impl;
+  errorcheck_functions.mutex_free = g_mutex_free_errorcheck_impl;
+    
+  g_thread_init (&errorcheck_functions);
+}
+
 void
 g_thread_init (GThreadFunctions* init)
 {
Index: gthread/gthread-posix.c
===================================================================
RCS file: /cvs/gnome/glib/gthread/gthread-posix.c,v
retrieving revision 1.20
diff -u -b -B -r1.20 gthread-posix.c
--- gthread/gthread-posix.c	2000/09/06 13:56:16	1.20
+++ gthread/gthread-posix.c	2000/09/22 13:38:21
@@ -73,6 +73,8 @@
 
 gulong g_thread_min_stack_size = 0;
 
+#define G_MUTEX_SIZE (sizeof (pthread_mutex_t))
+
 #define HAVE_G_THREAD_IMPL_INIT
 static void 
 g_thread_impl_init()
Index: gthread/gthread-solaris.c
===================================================================
RCS file: /cvs/gnome/glib/gthread/gthread-solaris.c,v
retrieving revision 1.12
diff -u -b -B -r1.12 gthread-solaris.c
--- gthread/gthread-solaris.c	2000/09/06 13:56:16	1.12
+++ gthread/gthread-solaris.c	2000/09/22 13:38:21
@@ -51,6 +51,8 @@
 
 gulong g_thread_min_stack_size = 0;
 
+#define G_MUTEX_SIZE (sizeof (mutex_t))
+
 #define HAVE_G_THREAD_IMPL_INIT
 static void 
 g_thread_impl_init()
Index: tests/thread-test.c
===================================================================
RCS file: /cvs/gnome/glib/tests/thread-test.c,v
retrieving revision 1.5
diff -u -b -B -r1.5 thread-test.c
--- tests/thread-test.c	2000/09/06 13:56:17	1.5
+++ tests/thread-test.c	2000/09/22 13:38:21
@@ -25,6 +25,7 @@
 
   g_assert (g_mutex_trylock (test_g_mutex_mutex));
   g_assert (G_TRYLOCK (test_g_mutex));
+  g_assert (G_TRYLOCK (test_g_mutex));
   thread = g_thread_create (test_g_mutex_thread, 
 			    GINT_TO_POINTER (42),
 			    0, TRUE, TRUE, G_THREAD_PRIORITY_NORMAL, NULL);