[glib: 2/3] gthread: Rework to avoid holding a mutex half the time

[Philip Withnall <pwithnall src gnome org>]

commit 630fa82ed0d8bde0014201df84aeed5fcd489667
Author: Colin Walters <walters verbum org>
Date:   Thu Nov 17 17:17:30 2016 -0500

    gthread: Rework to avoid holding a mutex half the time
    
    This code was a persistent source of `-fsanitize=thread` errors
    when I was trying to use it on OSTree.
    
    The problem is that while I think this code is functionally correct,
    we hold a mutex during the writes, but not the reads, and TSAN (IMO
    correctly) flags that.
    
    Reading this, I don't see a reason we need a mutex at all.  At the
    cost of some small code duplication between posix/win32, we can just
    pass the data we need down into each implementation.  This ends up
    being notably cleaner I think than the awkward "lock/unlock to
    serialize" dance.
    
    (Minor review changes made by Philip Withnall <withnall endlessm com>.)
    
    https://gitlab.gnome.org/GNOME/glib/issues/1224

 glib/gthread-posix.c  | 15 +++++++++++++--
 glib/gthread-win32.c  | 15 +++++++++++++--
 glib/gthread.c        | 28 ++--------------------------
 glib/gthreadprivate.h |  5 ++++-
 4 files changed, 32 insertions(+), 31 deletions(-)
---
diff --git a/glib/gthread-posix.c b/glib/gthread-posix.c
index 5fff51477..9c0b032e6 100644
--- a/glib/gthread-posix.c
+++ b/glib/gthread-posix.c
@@ -1148,15 +1148,26 @@ g_system_thread_free (GRealThread *thread)
 }
 
 GRealThread *
-g_system_thread_new (GThreadFunc   thread_func,
+g_system_thread_new (GThreadFunc   proxy,
                      gulong        stack_size,
+                     const char   *name,
+                     GThreadFunc   func,
+                     gpointer      data,
                      GError      **error)
 {
   GThreadPosix *thread;
+  GRealThread *base_thread;
   pthread_attr_t attr;
   gint ret;
 
   thread = g_slice_new0 (GThreadPosix);
+  base_thread = (GRealThread*)thread;
+  base_thread->ref_count = 2;
+  base_thread->ours = TRUE;
+  base_thread->thread.joinable = TRUE;
+  base_thread->thread.func = func;
+  base_thread->thread.data = data;
+  base_thread->name = g_strdup (name);
 
   posix_check_cmd (pthread_attr_init (&attr));
 
@@ -1174,7 +1185,7 @@ g_system_thread_new (GThreadFunc   thread_func,
     }
 #endif /* HAVE_PTHREAD_ATTR_SETSTACKSIZE */
 
-  ret = pthread_create (&thread->system_thread, &attr, (void* (*)(void*))thread_func, thread);
+  ret = pthread_create (&thread->system_thread, &attr, (void* (*)(void*))proxy, thread);
 
   posix_check_cmd (pthread_attr_destroy (&attr));
 
diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c
index 1ad5ece80..89df0daa0 100644
--- a/glib/gthread-win32.c
+++ b/glib/gthread-win32.c
@@ -429,15 +429,26 @@ g_thread_win32_proxy (gpointer data)
 }
 
 GRealThread *
-g_system_thread_new (GThreadFunc   func,
+g_system_thread_new (GThreadFunc   proxy,
                      gulong        stack_size,
+                     const char   *name,
+                     GThreadFunc   func,
+                     gpointer      data,
                      GError      **error)
 {
   GThreadWin32 *thread;
+  GRealThread *base_thread;
   guint ignore;
 
   thread = g_slice_new0 (GThreadWin32);
-  thread->proxy = func;
+  thread->proxy = proxy;
+  base_thread = (GRealThread*)thread;
+  base_thread->ref_count = 2;
+  base_thread->ours = TRUE;
+  base_thread->thread.joinable = TRUE;
+  base_thread->thread.func = func;
+  base_thread->thread.data = data;
+  base_thread->name = g_strdup (name);
 
   thread->handle = (HANDLE) _beginthreadex (NULL, stack_size, g_thread_win32_proxy, thread, 0, &ignore);
 
diff --git a/glib/gthread.c b/glib/gthread.c
index 2b7be91dd..66e4a1f66 100644
--- a/glib/gthread.c
+++ b/glib/gthread.c
@@ -515,8 +515,6 @@ static GSList   *g_once_init_list = NULL;
 static void g_thread_cleanup (gpointer data);
 static GPrivate     g_thread_specific_private = G_PRIVATE_INIT (g_thread_cleanup);
 
-G_LOCK_DEFINE_STATIC (g_thread_new);
-
 /*
  * g_private_set_alloc0:
  * @key: a #GPrivate
@@ -792,16 +790,8 @@ g_thread_proxy (gpointer data)
   GRealThread* thread = data;
 
   g_assert (data);
-
-  /* This has to happen before G_LOCK, as that might call g_thread_self */
   g_private_set (&g_thread_specific_private, data);
 
-  /* The lock makes sure that g_thread_new_internal() has a chance to
-   * setup 'func' and 'data' before we make the call.
-   */
-  G_LOCK (g_thread_new);
-  G_UNLOCK (g_thread_new);
-
   TRACE (GLIB_THREAD_SPAWNED (thread->thread.func, thread->thread.data,
                               thread->name));
 
@@ -897,24 +887,10 @@ g_thread_new_internal (const gchar   *name,
                        gsize          stack_size,
                        GError       **error)
 {
-  GRealThread *thread;
-
   g_return_val_if_fail (func != NULL, NULL);
 
-  G_LOCK (g_thread_new);
-  thread = g_system_thread_new (proxy, stack_size, error);
-  if (thread)
-    {
-      thread->ref_count = 2;
-      thread->ours = TRUE;
-      thread->thread.joinable = TRUE;
-      thread->thread.func = func;
-      thread->thread.data = data;
-      thread->name = g_strdup (name);
-    }
-  G_UNLOCK (g_thread_new);
-
-  return (GThread*) thread;
+  return (GThread*) g_system_thread_new (proxy, stack_size, name,
+                                         func, data, error);
 }
 
 /**
diff --git a/glib/gthreadprivate.h b/glib/gthreadprivate.h
index 75bb3cbf2..b0294af4c 100644
--- a/glib/gthreadprivate.h
+++ b/glib/gthreadprivate.h
@@ -37,8 +37,11 @@ struct  _GRealThread
 /* system thread implementation (gthread-posix.c, gthread-win32.c) */
 void            g_system_thread_wait            (GRealThread  *thread);
 
-GRealThread *   g_system_thread_new             (GThreadFunc   func,
+GRealThread *   g_system_thread_new             (GThreadFunc   proxy,
                                                  gulong        stack_size,
+                                                 const char   *name,
+                                                 GThreadFunc   func,
+                                                 gpointer      data,
                                                  GError      **error);
 void            g_system_thread_free            (GRealThread  *thread);