prevent race covered by g_once_init_enter(), by checking for previous

Tue Aug 14 02:06:10 2007  Tim Janik  <timj@imendio.com>

        * glib/gthread.c (g_once_init_enter_impl): prevent race covered
        by g_once_init_enter(), by checking for previous initializations
        before entering initialisation branch.

        * tests/onceinit.c: added multi-thread/multi-initializer stress test
        using unoptimized g_once_init_enter_impl().

svn path=/trunk/; revision=5701

diff --git a/ChangeLog b/ChangeLog
index 09a5510..37295d5 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+Tue Aug 14 02:06:10 2007  Tim Janik  <timj@imendio.com>
+
+       * glib/gthread.c (g_once_init_enter_impl): prevent race covered
+       by g_once_init_enter(), by checking for previous initializations
+       before entering initialisation branch.
+
+       * tests/onceinit.c: added multi-thread/multi-initializer stress test
+       using unoptimized g_once_init_enter_impl().
+
 Mon Aug 13 14:30:15 2007  Tim Janik  <timj@imendio.com>
 
        * tests/onceinit.c (main): fixed array size typo.

diff --git a/glib/gthread.c b/glib/gthread.c
index 5468e59..fdff396 100644 (file)
--- a/glib/gthread.c
+++ b/glib/gthread.c
@@ -202,18 +202,19 @@ g_once_impl (GOnce       *once,
 gboolean
 g_once_init_enter_impl (volatile gsize *value_location)
 {
-  gboolean need_init;
+  gboolean need_init = FALSE;
   g_mutex_lock (g_once_mutex);
-  if (!g_once_init_list || !g_slist_find (g_once_init_list, (void*) value_location))
+  if (g_atomic_pointer_get ((void**) value_location) == 0)
     {
-      g_once_init_list = g_slist_prepend (g_once_init_list, (void*) value_location);
-      need_init = TRUE;
-    }
-  else
-    {
-      while (g_slist_find (g_once_init_list, (void*) value_location))
-        g_cond_wait (g_once_cond, g_once_mutex);
-      need_init = FALSE;
+      if (!g_slist_find (g_once_init_list, (void*) value_location))
+        {
+          need_init = TRUE;
+          g_once_init_list = g_slist_prepend (g_once_init_list, (void*) value_location);
+        }
+      else
+        do
+          g_cond_wait (g_once_cond, g_once_mutex);
+        while (g_slist_find (g_once_init_list, (void*) value_location));
     }
   g_mutex_unlock (g_once_mutex);
   return need_init;

diff --git a/glib/gthread.h b/glib/gthread.h
index 3aaf14e..ea801fb 100644 (file)
--- a/glib/gthread.h
+++ b/glib/gthread.h
@@ -332,7 +332,7 @@ void                    g_once_init_leave       (volatile gsize *value_location,
 G_INLINE_FUNC gboolean
 g_once_init_enter (volatile gsize *value_location)
 {
-  if G_LIKELY (g_atomic_pointer_get ((void**) value_location) !=0)
+  if G_LIKELY (g_atomic_pointer_get ((void**) value_location) != 0)
     return FALSE;
   else
     return g_once_init_enter_impl (value_location);

diff --git a/tests/onceinit.c b/tests/onceinit.c
index 218ab39..372eb2f 100644 (file)
--- a/tests/onceinit.c
+++ b/tests/onceinit.c
@@ -21,11 +21,11 @@
 #include <glib.h>
 #include <stdlib.h>
 
-#define N_THREADS               (11)
+#define N_THREADS               (13)
 
 static GMutex      *tmutex = NULL;
 static GCond       *tcond = NULL;
-static volatile int tmain_call_count = 0;
+static volatile int thread_call_count = 0;
 static char         dummy_value = 'x';
 
 static void
@@ -102,10 +102,12 @@ tmain_call_initializer3 (gpointer user_data)
   //g_printf ("[");
   initializer3();
   //g_printf ("]\n");
-  g_atomic_int_exchange_and_add (&tmain_call_count, 1);
+  g_atomic_int_exchange_and_add (&thread_call_count, 1);
   return NULL;
 }
 
+static void*     stress_concurrent_initializers (void*);
+
 int
 main (int   argc,
       char *argv[])
@@ -132,7 +134,7 @@ main (int   argc,
   /* concurrently call initializer3() */
   g_cond_broadcast (tcond);
   /* loop until all threads passed the call to initializer3() */
-  while (g_atomic_int_get (&tmain_call_count) < i)
+  while (g_atomic_int_get (&thread_call_count) < i)
     {
       if (rand() % 2)
         g_thread_yield();   /* concurrent shuffling for single core */
@@ -140,5 +142,133 @@ main (int   argc,
         g_usleep (1000);    /* concurrent shuffling for multi core */
       g_cond_broadcast (tcond);
     }
+  /* call multiple (unoptimized) initializers from multiple threads */
+  g_mutex_lock (tmutex);
+  g_atomic_int_set (&thread_call_count, 0);
+  for (i = 0; i < N_THREADS; i++)
+    g_thread_create (stress_concurrent_initializers, 0, FALSE, NULL);
+  g_mutex_unlock (tmutex);
+  while (g_atomic_int_get (&thread_call_count) < 256 * 4 * N_THREADS)
+    g_usleep (50 * 1000);       /* wait for all 5 threads to complete */
   return 0;
 }
+
+/* get rid of g_once_init_enter-optimizations in the below definitions
+ * to uncover possible races in the g_once_init_enter_impl()/
+ * g_once_init_leave() implementations
+ */
+#define g_once_init_enter       g_once_init_enter_impl
+
+/* define 16 * 16 simple initializers */
+#define DEFINE_TEST_INITIALIZER(N)                      \
+      static void                                       \
+      test_initializer_##N (void)                       \
+      {                                                 \
+        static volatile gsize initialized = 0;          \
+        if (g_once_init_enter (&initialized))           \
+          {                                             \
+            g_free (g_strdup_printf ("cpuhog%5d", 1));  \
+            g_free (g_strdup_printf ("cpuhog%6d", 2));  \
+            g_free (g_strdup_printf ("cpuhog%7d", 3));  \
+            g_once_init_leave (&initialized, 1);        \
+          }                                             \
+      }
+#define DEFINE_16_TEST_INITIALIZERS(P)                  \
+                DEFINE_TEST_INITIALIZER (P##0)          \
+                DEFINE_TEST_INITIALIZER (P##1)          \
+                DEFINE_TEST_INITIALIZER (P##2)          \
+                DEFINE_TEST_INITIALIZER (P##3)          \
+                DEFINE_TEST_INITIALIZER (P##4)          \
+                DEFINE_TEST_INITIALIZER (P##5)          \
+                DEFINE_TEST_INITIALIZER (P##6)          \
+                DEFINE_TEST_INITIALIZER (P##7)          \
+                DEFINE_TEST_INITIALIZER (P##8)          \
+                DEFINE_TEST_INITIALIZER (P##9)          \
+                DEFINE_TEST_INITIALIZER (P##a)          \
+                DEFINE_TEST_INITIALIZER (P##b)          \
+                DEFINE_TEST_INITIALIZER (P##c)          \
+                DEFINE_TEST_INITIALIZER (P##d)          \
+                DEFINE_TEST_INITIALIZER (P##e)          \
+                DEFINE_TEST_INITIALIZER (P##f)
+#define DEFINE_256_TEST_INITIALIZERS(P)                 \
+                DEFINE_16_TEST_INITIALIZERS (P##_0)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_1)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_2)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_3)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_4)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_5)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_6)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_7)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_8)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_9)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_a)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_b)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_c)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_d)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_e)     \
+                DEFINE_16_TEST_INITIALIZERS (P##_f)
+
+/* list 16 * 16 simple initializers */
+#define LIST_16_TEST_INITIALIZERS(P)                    \
+                test_initializer_##P##0,                \
+                test_initializer_##P##1,                \
+                test_initializer_##P##2,                \
+                test_initializer_##P##3,                \
+                test_initializer_##P##4,                \
+                test_initializer_##P##5,                \
+                test_initializer_##P##6,                \
+                test_initializer_##P##7,                \
+                test_initializer_##P##8,                \
+                test_initializer_##P##9,                \
+                test_initializer_##P##a,                \
+                test_initializer_##P##b,                \
+                test_initializer_##P##c,                \
+                test_initializer_##P##d,                \
+                test_initializer_##P##e,                \
+                test_initializer_##P##f
+#define LIST_256_TEST_INITIALIZERS(P)                   \
+                LIST_16_TEST_INITIALIZERS (P##_0),      \
+                LIST_16_TEST_INITIALIZERS (P##_1),      \
+                LIST_16_TEST_INITIALIZERS (P##_2),      \
+                LIST_16_TEST_INITIALIZERS (P##_3),      \
+                LIST_16_TEST_INITIALIZERS (P##_4),      \
+                LIST_16_TEST_INITIALIZERS (P##_5),      \
+                LIST_16_TEST_INITIALIZERS (P##_6),      \
+                LIST_16_TEST_INITIALIZERS (P##_7),      \
+                LIST_16_TEST_INITIALIZERS (P##_8),      \
+                LIST_16_TEST_INITIALIZERS (P##_9),      \
+                LIST_16_TEST_INITIALIZERS (P##_a),      \
+                LIST_16_TEST_INITIALIZERS (P##_b),      \
+                LIST_16_TEST_INITIALIZERS (P##_c),      \
+                LIST_16_TEST_INITIALIZERS (P##_d),      \
+                LIST_16_TEST_INITIALIZERS (P##_e),      \
+                LIST_16_TEST_INITIALIZERS (P##_f)
+
+/* define 4 * 256 initializers */
+DEFINE_256_TEST_INITIALIZERS (stress1);
+DEFINE_256_TEST_INITIALIZERS (stress2);
+DEFINE_256_TEST_INITIALIZERS (stress3);
+DEFINE_256_TEST_INITIALIZERS (stress4);
+
+/* call the above 1024 initializers */
+static void*
+stress_concurrent_initializers (void *user_data)
+{
+  static void (*initializers[]) (void) = {
+    LIST_256_TEST_INITIALIZERS (stress1),
+    LIST_256_TEST_INITIALIZERS (stress2),
+    LIST_256_TEST_INITIALIZERS (stress3),
+    LIST_256_TEST_INITIALIZERS (stress4),
+  };
+  int i;
+  /* sync to main thread */
+  g_mutex_lock (tmutex);
+  g_mutex_unlock (tmutex);
+  /* initialize concurrently */
+  for (i = 0; i < G_N_ELEMENTS (initializers); i++)
+    {
+      initializers[i]();
+      g_atomic_int_exchange_and_add (&thread_call_count, 1);
+    }
+  return NULL;
+}