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

@@ -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));
 

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);
 

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);
 }
 
 /**

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);