GNOME Bugzilla – Bug 556515
g_object_unref race condition
Last modified: 2009-10-06 16:32:47 UTC
Steps to reproduce: 1. Have a GObject 'obj' shared between two threads A and B. The ref count of 'obj' is 2. 2. Thread A and B does 'g_object_unref (obj);' at the same time. Given an execution order where A gets the first unref CAS and suspends after "gobject.c:2380". Thread B then completes the last unref finalizes 'obj' and calls free. Thread A then continues and tries to access the memory previously held by object (gobject.c:2382). 3. If by chance the memory accessed contains a 1 in bit 0, additional code will be called (toggle_refs_notify gobject.c:2383) blowing up further . Stack trace: Other information: This is on Windows XP, thus the crash depends on the C runtime stamping the freed memory with 0xdd. We have a test case that we will append. The fix is probably to move the reading of the OBJECT_HAS_TOGGLE_REF before the CAS. Additionally the same bug seems to lurk a bit further down where OBJECT_HAS_TOGGLE_REF is also accessed after a CAS.
Stack Trace: > libglib-2.0-0.dll!g_datalist_id_get_data(_GData * * datalist=0x004168e0, unsigned int key_id=51) Line 444 + 0x3 bytes C libgobject-2.0-0.dll!toggle_refs_notify(_GObject * object=0x004168d8, int is_last_ref=1) Line 2201 + 0x12 bytes C libgobject-2.0-0.dll!g_object_unref(void * _object=0x004168d8) Line 2405 + 0xb bytes C gobjectunreftest.exe!slow_worker_func(void * data=0x004168d8) Line 78 + 0x9 bytes C libglib-2.0-0.dll!g_thread_create_proxy(void * data=0x00416ca8) Line 644 + 0x10 bytes C libgthread-2.0-0.dll!g_thread_proxy(void * data=0x00416d10) Line 478 + 0x10 bytes C msvcr80d.dll!_callthreadstartex() Line 348 + 0xf bytes C msvcr80d.dll!_threadstartex(void * ptd=0x004172b8) Line 331 C kernel32.dll!7c80b713() [Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]
Created attachment 120699 [details] Test case This test triggers the bug if a "g_usleep (G_USEC_PER_SEC/2);" is patched in gobject.c: === modified file 'gobject/gobject.c' --- gobject/gobject.c 2008-09-16 15:53:43 +0000 +++ gobject/gobject.c 2008-10-16 09:53:36 +0000 @@ -2398,6 +2398,8 @@ if (!g_atomic_int_compare_and_exchange (&object->ref_count, old_ref, old_ref - 1)) goto retry_atomic_decrement1; + g_usleep (G_USEC_PER_SEC / 2); + /* if we went from 2->1 we need to notify toggle refs if any */ if (old_ref == 2 && OBJECT_HAS_TOGGLE_REF (object)) toggle_refs_notify (object, TRUE);
CC otaylor as this was introduced with toggle_ref.
Created attachment 126094 [details] [review] g_object_unref_race_fix Jarl Christian's suggested fix.
can someone review the patch, please?
This is a duplicate of bug #551706
Yes, this is a duplicate, both bugs even have very similar patches.
(In reply to comment #4) > Created an attachment (id=126094) [details] > g_object_unref_race_fix > > Jarl Christian's suggested fix. I've rejected this patch, because the old_ref==2 needs to stay in the original place. Bug #551706 has the proper fix for this issue. However, this bug may help in testing as the other bug doesn#t have a test case attached. Cody, can you please test the fix with the test from this report?
*** This bug has been marked as a duplicate of bug 551706 ***