GNOME Bugzilla – Bug 587892
[PATCH] Race in GType when instantiating the same class for the first time from multiple threads.
Last modified: 2009-12-01 08:01:28 UTC
Steps to reproduce: The problem is that the g_type_class_peek function may return a GTypeClass not yet fully initialized, (class_init() has not yet been called or finished executing) meaning it might call virtual methods on the base class instead of the derived class (like GObjectClass::constructor(), making the object not construct properly). The fix is to let g_type_class_peek make sure the class is fully constructed (checking that class.init_state == INITIALIZED), since this flag is only set when the GTypeClass has been fully initialized. The only possible side-effects of this fix would be if g_type_class_peek were called inside the class_init(), it would return NULL, and if some implementation expects this to not fail, this patch will break that. However, it does not quite make sense that a class should be ready for "peeking" while it is still under construction, so the fix should be safe. See test case to be attached. Stack trace: Other information:
Created attachment 137920 [details] [review] Proposed fix by Benjamin Otte (Company)
Created attachment 137921 [details] Test case that easily reproduces the issue What we do here is to construct the object type from 4 different threads more or less at the same time, but add a little sleep inside the class_init(), making the race more likely to occur.
The patch makes a lot of sense to me. Tim, do you agree that this is correct ? Thanks for the testcase. It would be nice to have a patch that integrates the testcase in gobject/tests/Makefile.am.
(In reply to comment #0) > The only possible side-effects of this fix would be if g_type_class_peek were > called inside the class_init(), it would return NULL, and if some > implementation expects this to not fail, this patch will break that. Unfortunately, this is exactly the use case that the current code means to still support. Mostly for backward compatibility reasons. IIRC, when GType was written, old GtkType code did peek classes during construction, so gtype.c was written to support this. I don't have a concrete usage case in mind atm though.
I believe this has been fixed by the gobject-performance merge. If it turns out there are cases where we need to support peeking during construction, then we have to allow that but get rid of using g_type_class_peek_static() in g_object_new() and use g_type_class_ref() unconditionally - that should not be a big performance hit with the patches merged.