From 191c80c9dec3b1dc48cdf59eba699c0edff706ff Mon Sep 17 00:00:00 2001 From: timj Date: Tue, 10 Jun 2008 11:26:50 +0000 Subject: [PATCH 1/2] 2008-06-10 13:15:29 Tim Janik * gtype.c (g_type_class_ref): fixed race condition where references to partially initialized classes could be handed out. git-svn-id: svn+ssh://svn.gnome.org/svn/glib/trunk@6982 5bbd4a9e-d125-0410-bf1d-f987e7eefc80 --- gobject/ChangeLog | 5 +++++ gobject/gtype.c | 47 ++++++++++++++++++++--------------------------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/gobject/gtype.c b/gobject/gtype.c index 8c8fc41..143e195 100644 --- a/gobject/gtype.c +++ b/gobject/gtype.c @@ -2363,20 +2363,19 @@ gpointer g_type_class_ref (GType type) { TypeNode *node; - - /* optimize for common code path - */ + GType ptype; + + /* optimize for common code path */ G_WRITE_LOCK (&type_rw_lock); node = lookup_type_node_I (type); if (node && node->is_classed && node->data && - node->data->class.class && node->data->common.ref_count > 0) + node->data->class.class && + node->data->class.init_state == INITIALIZED) { type_data_ref_Wm (node); G_WRITE_UNLOCK (&type_rw_lock); - return node->data->class.class; } - if (!node || !node->is_classed || (node->data && node->data->common.ref_count < 1)) { @@ -2385,33 +2384,27 @@ g_type_class_ref (GType type) type_descriptive_name_I (type)); return NULL; } - type_data_ref_Wm (node); + ptype = NODE_PARENT_TYPE (node); + G_WRITE_UNLOCK (&type_rw_lock); + g_static_rec_mutex_lock (&class_init_rec_mutex); /* required locking order: 1) class_init_rec_mutex, 2) type_rw_lock */ + /* here, we either have node->data->class.class == NULL, or + * node->data->class.init_state == INITIALIZED, because any + * concurrently running initialization was guarded by class_init_rec_mutex. + */ if (!node->data->class.class) /* class uninitialized */ { - GType ptype = NODE_PARENT_TYPE (node); - GTypeClass *pclass = NULL; - G_WRITE_UNLOCK (&type_rw_lock); - g_static_rec_mutex_lock (&class_init_rec_mutex); /* required locking order: 1) class_init_rec_mutex, 2) type_rw_lock */ - if (ptype) - { - pclass = g_type_class_ref (ptype); - G_WRITE_LOCK (&type_rw_lock); - node = lookup_type_node_I (type); - if (node->data->class.class) - INVALID_RECURSION ("g_type_plugin_*", node->plugin, NODE_NAME (node)); - } - else - { - G_WRITE_LOCK (&type_rw_lock); - node = lookup_type_node_I (type); - } - if (!node->data->class.class) /* class could have been initialized meanwhile */ - type_class_init_Wm (node, pclass); + /* acquire reference on parent class */ + GTypeClass *pclass = ptype ? g_type_class_ref (ptype) : NULL; + G_WRITE_LOCK (&type_rw_lock); + if (node->data->class.class) /* class was initialized during parent class initialization? */ + INVALID_RECURSION ("g_type_plugin_*", node->plugin, NODE_NAME (node)); + type_class_init_Wm (node, pclass); G_WRITE_UNLOCK (&type_rw_lock); - g_static_rec_mutex_unlock (&class_init_rec_mutex); } + g_static_rec_mutex_unlock (&class_init_rec_mutex); + return node->data->class.class; } -- 1.5.4.5 From 21706a75fdcad8691095f1157216c664cda46f1f Mon Sep 17 00:00:00 2001 From: timj Date: Tue, 10 Jun 2008 11:35:46 +0000 Subject: [PATCH 2/2] 2008-06-10 13:34:01 Tim Janik * tests/threadtests.c: added race condition tester from Michael Meeks with a couple fixes so it's not triggering development warnings. From: Bug 537555 - GObject instantiation not thread safe ... git-svn-id: svn+ssh://svn.gnome.org/svn/glib/trunk@6983 5bbd4a9e-d125-0410-bf1d-f987e7eefc80 --- gobject/ChangeLog | 6 +++ gobject/tests/threadtests.c | 75 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 80 insertions(+), 1 deletions(-) diff --git a/gobject/tests/threadtests.c b/gobject/tests/threadtests.c index 3b95503..eabbe05 100644 --- a/gobject/tests/threadtests.c +++ b/gobject/tests/threadtests.c @@ -109,6 +109,7 @@ G_DEFINE_TYPE_WITH_CODE (MyTester2, my_tester2, G_TYPE_OBJECT, static void my_tester2_init (MyTester2*t) {} static void my_tester2_class_init (MyTester2Class*c) { call_counter_init (c); } +static GCond *sync_cond = NULL; static GMutex *sync_mutex = NULL; static gpointer @@ -138,7 +139,6 @@ test_threaded_class_init (void) { GThread *threads[3] = { NULL, }; /* pause newly created threads */ - sync_mutex = g_mutex_new(); g_mutex_lock (sync_mutex); /* create threads */ threads[0] = g_thread_create (tester_init_thread, (gpointer) my_tester0_get_type(), TRUE, NULL); @@ -158,6 +158,75 @@ test_threaded_class_init (void) g_assert_cmpint (g_atomic_int_get (&mtsafe_call_counter), ==, unsafe_call_counter); } +typedef struct { + GObject parent; + char *name; +} PropTester; +typedef GObjectClass PropTesterClass; +G_DEFINE_TYPE (PropTester, prop_tester, G_TYPE_OBJECT); +#define PROP_NAME 1 +static void +prop_tester_init (PropTester* t) +{ + if (t->name == NULL) + ; // neds unit test framework initialization: g_test_bug ("race initializing properties"); +} +static void +prop_tester_set_property (GObject *object, + guint property_id, + const GValue *value, + GParamSpec *pspec) +{} +static void +prop_tester_class_init (PropTesterClass *c) +{ + int i; + GParamSpec *param; + GObjectClass *gobject_class = G_OBJECT_CLASS (c); + + gobject_class->set_property = prop_tester_set_property; /* silence GObject checks */ + + g_mutex_lock (sync_mutex); + g_cond_signal (sync_cond); + g_mutex_unlock (sync_mutex); + + for (i = 0; i < 100; i++) /* wait a bit. */ + g_thread_yield(); + + call_counter_init (c); + param = g_param_spec_string ("name", "name_i18n", + "yet-more-wasteful-i18n", + NULL, + G_PARAM_CONSTRUCT_ONLY | G_PARAM_WRITABLE | + G_PARAM_STATIC_NAME | G_PARAM_STATIC_BLURB | + G_PARAM_STATIC_NICK); + g_object_class_install_property (gobject_class, PROP_NAME, param); +} + +static gpointer +object_create (gpointer data) +{ + GObject *obj = g_object_new (prop_tester_get_type(), "name", "fish", NULL); + g_object_unref (obj); + return NULL; +} + +static void +test_threaded_object_init (void) +{ + GThread *creator; + g_mutex_lock (sync_mutex); + + creator = g_thread_create (object_create, NULL, TRUE, NULL); + /* really provoke the race */ + g_cond_wait (sync_cond, sync_mutex); + + object_create (NULL); + g_mutex_unlock (sync_mutex); + + g_thread_join (creator); +} + int main (int argc, char *argv[]) @@ -166,7 +235,11 @@ main (int argc, g_test_init (&argc, &argv, NULL); g_type_init (); + sync_cond = g_cond_new(); + sync_mutex = g_mutex_new(); + g_test_add_func ("/GObject/threaded-class-init", test_threaded_class_init); + g_test_add_func ("/GObject/threaded-object-init", test_threaded_object_init); return g_test_run(); } -- 1.5.4.5