Make the magic and location arguments to the error-checking-mutex

2006-05-10  Sebastian Wilhelmi  <wilhelmi@google.com>

	* glib/gthread.h, gthread/gthread-impl.c: Make the magic and
	location arguments to the error-checking-mutex functions const and
	do not write to them, as we might not own them. Clean up the
	error-checking-mutex code quite a bit. (#335198, Chris Wilson)

diff --git a/ChangeLog b/ChangeLog
index 3694f49..b5dd950 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2006-05-10  Sebastian Wilhelmi  <wilhelmi@google.com>
 
+       * glib/gthread.h, gthread/gthread-impl.c: Make the magic and
+       location arguments to the error-checking-mutex functions const and
+       do not write to them, as we might not own them. Clean up the
+       error-checking-mutex code quite a bit. (#335198, Chris Wilson)
+
        * glib/gthread.c: Use g_atomic_pointer_set instead of old
        homegrown version now that we have it. (#335198, Chris Wilson)
 

diff --git a/ChangeLog.pre-2-12 b/ChangeLog.pre-2-12
index 3694f49..b5dd950 100644 (file)
--- a/ChangeLog.pre-2-12
+++ b/ChangeLog.pre-2-12
@@ -1,5 +1,10 @@
 2006-05-10  Sebastian Wilhelmi  <wilhelmi@google.com>
 
+       * glib/gthread.h, gthread/gthread-impl.c: Make the magic and
+       location arguments to the error-checking-mutex functions const and
+       do not write to them, as we might not own them. Clean up the
+       error-checking-mutex code quite a bit. (#335198, Chris Wilson)
+
        * glib/gthread.c: Use g_atomic_pointer_set instead of old
        homegrown version now that we have it. (#335198, Chris Wilson)
 

diff --git a/glib/gthread.h b/glib/gthread.h
index 2ae313f..377e8e5 100644 (file)
--- a/glib/gthread.h
+++ b/glib/gthread.h
@@ -148,7 +148,8 @@ GMutex* g_static_mutex_get_mutex_impl   (GMutex **mutex);
 #define G_THREAD_CF(op, fail, arg)                                     \
     (g_thread_supported () ? G_THREAD_UF (op, arg) : (fail))
 #define G_THREAD_ECF(op, fail, mutex, type)                            \
-    (g_thread_supported () ? ((type(*)(GMutex*, gulong, gchar*))       \
+    (g_thread_supported () ?                                           \
+      ((type(*)(GMutex*, const gulong, gchar const*))                  \
       (*g_thread_functions_for_glib_use . op))                         \
      (mutex, G_MUTEX_DEBUG_MAGIC, G_STRLOC) : (fail))
 
@@ -374,4 +375,3 @@ extern void glib_dummy_decl (void);
 G_END_DECLS
 
 #endif /* __G_THREAD_H__ */
-

diff --git a/gthread/gthread-impl.c b/gthread/gthread-impl.c
index 89e7ffc..77615ce 100644 (file)
--- a/gthread/gthread-impl.c
+++ b/gthread/gthread-impl.c
@@ -24,10 +24,10 @@
  * Modified by the GLib Team and others 1997-2000.  See the AUTHORS
  * file for a list of people on the GLib Team.  See the ChangeLog
  * files for a list of changes.  These files are distributed with
- * GLib at ftp://ftp.gtk.org/pub/gtk/. 
+ * GLib at ftp://ftp.gtk.org/pub/gtk/.
  */
 
-/* 
+/*
  * MT safe
  */
 
@@ -69,119 +69,95 @@ void g_convert_init (void);
 void g_rand_init (void);
 void g_main_thread_init (void);
 
-#define G_MUTEX_DEBUG_INFO(mutex) (*((gpointer*)(((char*)mutex)+G_MUTEX_SIZE)))
-
-typedef struct _ErrorCheckInfo ErrorCheckInfo;
-struct _ErrorCheckInfo
+typedef struct _GMutexDebugInfo GMutexDebugInfo;
+struct _GMutexDebugInfo
 {
   gchar *location;
   GSystemThread owner;
 };
 
+#define G_MUTEX_DEBUG_INFO(mutex)                                      \
+  (((GMutexDebugInfo*)(((char*)mutex)+G_MUTEX_SIZE)))
+
 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;
+  GMutexDebugInfo *info;
+  retval = g_realloc (retval, G_MUTEX_SIZE + sizeof (GMutexDebugInfo));
+
+  info = G_MUTEX_DEBUG_INFO (retval);
+  g_system_thread_assign (info->owner, zero_thread);
+  info->location = "invalid";
+
   return retval;
 }
 
 static void
-g_mutex_lock_errorcheck_impl (GMutex *mutex, 
-                             gulong magic,
-                             gchar *location)
+g_mutex_lock_errorcheck_impl (GMutex *mutex,
+                             const gulong magic,
+                             gchar * const location)
 {
-  ErrorCheckInfo *info;
-  GSystemThread self;
+  GMutexDebugInfo *info = G_MUTEX_DEBUG_INFO (mutex);
+  gchar *loc = (magic == G_MUTEX_DEBUG_MAGIC) ? location : "unknown";
 
+  GSystemThread self;
   g_thread_functions_for_glib_use.thread_self (&self);
 
-  if (magic != G_MUTEX_DEBUG_MAGIC)
-    location = "unknown";
-
-  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 another thread might have
-       * tried to lock 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 (g_system_thread_equal (info->owner, self))
     g_error ("Trying to recursivly lock a mutex at '%s', "
-            "previously locked at '%s'", 
-            location, info->location);
+            "previously locked at '%s'",
+            loc, info->location);
 
   g_thread_functions_for_glib_use_default.mutex_lock (mutex);
 
   g_system_thread_assign (info->owner, self);
-  info->location = location;
+  info->location = loc;
 }
 
 static gboolean
-g_mutex_trylock_errorcheck_impl (GMutex *mutex, 
-                                gulong magic, 
-                                gchar *location)
+g_mutex_trylock_errorcheck_impl (GMutex *mutex,
+                                const gulong magic,
+                                gchar * const location)
 {
-  ErrorCheckInfo *info = G_MUTEX_DEBUG_INFO (mutex);
-  GSystemThread self;
+  GMutexDebugInfo *info = G_MUTEX_DEBUG_INFO (mutex);
+  gchar *loc = (magic == G_MUTEX_DEBUG_MAGIC) ? location : "unknown";
 
+  GSystemThread self;
   g_thread_functions_for_glib_use.thread_self (&self);
 
-  if (magic != G_MUTEX_DEBUG_MAGIC)
-    location = "unknown";
-
-  if (!info)
-    {
-      /* This mutex has not yet been used, so simply lock and return TRUE */
-      g_mutex_lock_errorcheck_impl (mutex, magic, location);
-      return TRUE;
-    }
-
   if (g_system_thread_equal (info->owner, self))
     g_error ("Trying to recursivly lock a mutex at '%s', "
-            "previously locked at '%s'", 
-            location, info->location);
-  
+            "previously locked at '%s'",
+            loc, info->location);
+
   if (!g_thread_functions_for_glib_use_default.mutex_trylock (mutex))
     return FALSE;
 
   g_system_thread_assign (info->owner, self);
-  info->location = location;
+  info->location = loc;
 
   return TRUE;
 }
 
 static void
-g_mutex_unlock_errorcheck_impl (GMutex *mutex, 
-                               gulong magic, 
-                               gchar *location)
+g_mutex_unlock_errorcheck_impl (GMutex *mutex,
+                               const gulong magic,
+                               gchar * const location)
 {
-  ErrorCheckInfo *info = G_MUTEX_DEBUG_INFO (mutex);
-  GSystemThread self;
+  GMutexDebugInfo *info = G_MUTEX_DEBUG_INFO (mutex);
+  gchar *loc = (magic == G_MUTEX_DEBUG_MAGIC) ? location : "unknown";
 
+  GSystemThread self;
   g_thread_functions_for_glib_use.thread_self (&self);
 
-  if (magic != G_MUTEX_DEBUG_MAGIC)
-    location = "unknown";
-
-  if (!info || g_system_thread_equal (info->owner, zero_thread))
-    g_error ("Trying to unlock an unlocked mutex at '%s'", location);
+  if (g_system_thread_equal (info->owner, zero_thread))
+    g_error ("Trying to unlock an unlocked mutex at '%s'", loc);
 
   if (!g_system_thread_equal (info->owner, self))
     g_warning ("Trying to unlock a mutex at '%s', "
               "previously locked by a different thread at '%s'",
-              location, info->location);
+              loc, info->location);
 
   g_system_thread_assign (info->owner, zero_thread);
   info->location = NULL;
@@ -190,90 +166,81 @@ g_mutex_unlock_errorcheck_impl (GMutex *mutex,
 }
 
 static void
-g_mutex_free_errorcheck_impl (GMutex *mutex, 
-                             gulong magic, 
-                             gchar *location)
+g_mutex_free_errorcheck_impl (GMutex *mutex,
+                             const gulong magic,
+                             gchar * const location)
 {
-  ErrorCheckInfo *info = G_MUTEX_DEBUG_INFO (mutex);
-  
-  if (magic != G_MUTEX_DEBUG_MAGIC)
-    location = "unknown";
+  GMutexDebugInfo *info = G_MUTEX_DEBUG_INFO (mutex);
+  gchar *loc = (magic == G_MUTEX_DEBUG_MAGIC) ? location : "unknown";
 
   if (info && !g_system_thread_equal (info->owner, zero_thread))
     g_error ("Trying to free a locked mutex at '%s', "
-            "which was previously locked at '%s'", 
-            location, info->location);
+            "which was previously locked at '%s'",
+            loc, info->location);
 
-  g_free (G_MUTEX_DEBUG_INFO (mutex));
-  g_thread_functions_for_glib_use_default.mutex_free (mutex);  
+  g_thread_functions_for_glib_use_default.mutex_free (mutex);
 }
 
-static void     
+static void
 g_cond_wait_errorcheck_impl (GCond *cond,
-                            GMutex *mutex, 
-                            gulong magic, 
-                            gchar *location)
+                            GMutex *mutex,
+                            const gulong magic,
+                            gchar * const location)
 {
-  
-  ErrorCheckInfo *info = G_MUTEX_DEBUG_INFO (mutex);
-  GSystemThread self;
+  GMutexDebugInfo *info = G_MUTEX_DEBUG_INFO (mutex);
+  gchar *loc = (magic == G_MUTEX_DEBUG_MAGIC) ? location : "unknown";
 
+  GSystemThread self;
   g_thread_functions_for_glib_use.thread_self (&self);
 
-  if (magic != G_MUTEX_DEBUG_MAGIC)
-    location = "unknown";
-
-  if (!info || g_system_thread_equal (info->owner, zero_thread))
-    g_error ("Trying to use an unlocked mutex in g_cond_wait() at '%s'",
-            location);
+  if (g_system_thread_equal (info->owner, zero_thread))
+    g_error ("Trying to use an unlocked mutex in g_cond_wait() at '%s'", loc);
 
   if (!g_system_thread_equal (info->owner, self))
     g_error ("Trying to use a mutex locked by another thread in "
-            "g_cond_wait() at '%s'", location);
+            "g_cond_wait() at '%s'", loc);
 
   g_system_thread_assign (info->owner, zero_thread);
-  location = info->location;
+  loc = info->location;
 
   g_thread_functions_for_glib_use_default.cond_wait (cond, mutex);
 
   g_system_thread_assign (info->owner, self);
-  info->location = location;
+  info->location = loc;
 }
-    
 
-static gboolean 
+
+static gboolean
 g_cond_timed_wait_errorcheck_impl (GCond *cond,
                                    GMutex *mutex,
-                                   GTimeVal *end_time, 
-                                  gulong magic, 
-                                  gchar *location)
+                                   GTimeVal *end_time,
+                                  const gulong magic,
+                                  gchar * const location)
 {
-  ErrorCheckInfo *info = G_MUTEX_DEBUG_INFO (mutex);
-  GSystemThread self;
+  GMutexDebugInfo *info = G_MUTEX_DEBUG_INFO (mutex);
+  gchar *loc = (magic == G_MUTEX_DEBUG_MAGIC) ? location : "unknown";
   gboolean retval;
 
+  GSystemThread self;
   g_thread_functions_for_glib_use.thread_self (&self);
 
-  if (magic != G_MUTEX_DEBUG_MAGIC)
-    location = "unknown";
-
-  if (!info || g_system_thread_equal (info->owner, zero_thread))
+  if (g_system_thread_equal (info->owner, zero_thread))
     g_error ("Trying to use an unlocked mutex in g_cond_timed_wait() at '%s'",
-            location);
+            loc);
 
   if (!g_system_thread_equal (info->owner, self))
     g_error ("Trying to use a mutex locked by another thread in "
-            "g_cond_timed_wait() at '%s'", location);
+            "g_cond_timed_wait() at '%s'", loc);
 
   g_system_thread_assign (info->owner, zero_thread);
-  location = info->location;
-  
-  retval = g_thread_functions_for_glib_use_default.cond_timed_wait (cond, 
-                                                                   mutex, 
+  loc = info->location;
+
+  retval = g_thread_functions_for_glib_use_default.cond_timed_wait (cond,
+                                                                   mutex,
                                                                    end_time);
 
   g_system_thread_assign (info->owner, self);
-  info->location = location;
+  info->location = loc;
 
   return retval;
 }
@@ -282,12 +249,12 @@ g_cond_timed_wait_errorcheck_impl (GCond *cond,
 /* unshadow function declaration. See gthread.h */
 #undef g_thread_init
 
-void 
+void
 g_thread_init_with_errorcheck_mutexes (GThreadFunctions* init)
 {
   GThreadFunctions errorcheck_functions;
   if (init)
-    g_error ("Errorcheck mutexes can only be used for native " 
+    g_error ("Errorcheck mutexes can only be used for native "
             "thread implementations. Sorry." );
 
 #ifdef HAVE_G_THREAD_IMPL_INIT
@@ -304,20 +271,20 @@ g_thread_init_with_errorcheck_mutexes (GThreadFunctions* init)
 
   errorcheck_functions = g_thread_functions_for_glib_use_default;
   errorcheck_functions.mutex_new = g_mutex_new_errorcheck_impl;
-  errorcheck_functions.mutex_lock = 
+  errorcheck_functions.mutex_lock =
     (void (*)(GMutex *)) g_mutex_lock_errorcheck_impl;
-  errorcheck_functions.mutex_trylock = 
+  errorcheck_functions.mutex_trylock =
     (gboolean (*)(GMutex *)) g_mutex_trylock_errorcheck_impl;
-  errorcheck_functions.mutex_unlock = 
+  errorcheck_functions.mutex_unlock =
     (void (*)(GMutex *)) g_mutex_unlock_errorcheck_impl;
-  errorcheck_functions.mutex_free = 
+  errorcheck_functions.mutex_free =
     (void (*)(GMutex *)) g_mutex_free_errorcheck_impl;
-  errorcheck_functions.cond_wait = 
+  errorcheck_functions.cond_wait =
     (void (*)(GCond *, GMutex *)) g_cond_wait_errorcheck_impl;
-  errorcheck_functions.cond_timed_wait = 
-    (gboolean (*)(GCond *, GMutex *, GTimeVal *)) 
+  errorcheck_functions.cond_timed_wait =
+    (gboolean (*)(GCond *, GMutex *, GTimeVal *))
     g_cond_timed_wait_errorcheck_impl;
-    
+
   g_thread_init (&errorcheck_functions);
 }
 
@@ -328,7 +295,7 @@ g_thread_init (GThreadFunctions* init)
 
   if (thread_system_already_initialized)
     g_error ("GThread system may only be initialized once.");
-    
+
   thread_system_already_initialized = TRUE;
 
   if (init == NULL)
@@ -346,15 +313,15 @@ g_thread_init (GThreadFunctions* init)
 
   g_thread_functions_for_glib_use = *init;
 
-  supported = (init->mutex_new &&  
-              init->mutex_lock && 
-              init->mutex_trylock && 
-              init->mutex_unlock && 
-              init->mutex_free && 
-              init->cond_new && 
-              init->cond_signal && 
-              init->cond_broadcast && 
-              init->cond_wait && 
+  supported = (init->mutex_new &&
+              init->mutex_lock &&
+              init->mutex_trylock &&
+              init->mutex_unlock &&
+              init->mutex_free &&
+              init->cond_new &&
+              init->cond_signal &&
+              init->cond_broadcast &&
+              init->cond_wait &&
               init->cond_timed_wait &&
               init->cond_free &&
               init->private_new &&