GNOME Bugzilla – Bug 537555
GObject instantiation not thread safe ...
Last modified: 2009-08-28 05:37:13 UTC
So - this is related to bug 64764, but different. The basic problem is that we can do: Foo *foo = g_object_new (foo_get_type(), "property", value, NULL); in two threads simultaneously. Indeed we see this happen regularly (and it happes suspiciously in various crashing applications), eg. gnome-screenshot: https://bugzilla.novell.com/show_bug.cgi?id=385128#c15 the file-selector does something like the above and provokes the issue, as you can see in the trace / output. The essential problem is that the construction time property "property" is not initialized by the time the the second thread starts to construct the object, so we get to: gobject.c (g_object_new_valist): class = g_type_class_ref (object_type); ... GParamSpec *pspec = g_param_spec_pool_lookup (pspec_pool, name, object_type, TRUE); if (!pspec) { g_warning ("%s: object class `%s' has no property named `%s'", G_STRFUNC, g_type_name (object_type), name); and give a warning: (firefox:14234): GLib-GObject-WARNING **: IA__g_object_new_valist: object class `GThemedIcon' has no property named `names' which is a nonsense: and if you break here - you discover that (as the other thread runs on) - you immediately have the property as you ask for it in the debugger there :-) [ unfortunately it's a rather narrow race & hard to catch in the debugger ].
Created attachment 112463 [details] [review] threadtests patch with test showing the problem
So - it seems to me that in g_type_class_ref - we really don't want to be handing out pointers to not-fully-initialized class data for a given type: it seems like rather a bad plan ;-) gtype.c (g_type_class_ref): if (!node->data->class.class) /* class uninitialized */ { ... g_static_rec_mutex_lock (&class_init_rec_mutex); ... g_static_rec_mutex_unlock (&class_init_rec_mutex); } return node->data->class.class; should really be some typical double-check locking type pattern, but of course currently is not - the assignment to class.class occurs extremely early in this section - where it should be a very late statement (right?).
Thanks, applied the test with some runtime warning fixes. Please diff -up for patches next time. The test runs without problems here after recent upstream race condition fixes. 2008-06-10 13:34:01 Tim Janik <timj@imendio.com> * 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 ...
(In reply to comment #2) > So - it seems to me that in g_type_class_ref - we really don't want to be > handing out pointers to not-fully-initialized class data for a given type: it > seems like rather a bad plan ;-) > > gtype.c (g_type_class_ref): > > if (!node->data->class.class) /* class uninitialized */ > { > ... > g_static_rec_mutex_lock (&class_init_rec_mutex); > ... > g_static_rec_mutex_unlock (&class_init_rec_mutex); > } > return node->data->class.class; > > should really be some typical double-check locking type pattern, but of course > currently is not - the assignment to class.class occurs extremely early in this > section - where it should be a very late statement (right?). Unfortunately not, as type_class_peek and recursive _ref() needs to work on partially initialized classes. That's why we use initi_state for proper guarding in recent upstream now.
*** Bug 535257 has been marked as a duplicate of this bug. ***
*** Bug 535295 has been marked as a duplicate of this bug. ***
*** Bug 546799 has been marked as a duplicate of this bug. ***
*** Bug 534292 has been marked as a duplicate of this bug. ***
*** Bug 519387 has been marked as a duplicate of this bug. ***
*** Bug 531814 has been marked as a duplicate of this bug. ***
*** Bug 532138 has been marked as a duplicate of this bug. ***