From d5c437081366ba9617c9a23a09d167aede52a8b9 Mon Sep 17 00:00:00 2001 From: Tim Janik Date: Tue, 14 Aug 2007 00:05:52 +0000 Subject: [PATCH] prevent race covered by g_once_init_enter(), by checking for previous Tue Aug 14 02:06:10 2007 Tim Janik * 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 --- ChangeLog | 9 ++++ glib/gthread.c | 21 +++++---- glib/gthread.h | 2 +- tests/onceinit.c | 138 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 155 insertions(+), 15 deletions(-) diff --git a/ChangeLog b/ChangeLog index 09a5510..37295d5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +Tue Aug 14 02:06:10 2007 Tim Janik + + * 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 * tests/onceinit.c (main): fixed array size typo. diff --git a/glib/gthread.c b/glib/gthread.c index 5468e59..fdff396 100644 --- 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 --- 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 --- a/tests/onceinit.c +++ b/tests/onceinit.c @@ -21,11 +21,11 @@ #include #include -#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; +} -- 2.7.4