After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 556515 - g_object_unref race condition
g_object_unref race condition
Status: RESOLVED DUPLICATE of bug 551706
Product: glib
Classification: Platform
Component: gobject
2.18.x
Other All
: High critical
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2008-10-16 09:03 UTC by Jarl Christian Berentsen
Modified: 2009-10-06 16:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case (1.63 KB, text/plain)
2008-10-16 10:00 UTC, Jarl Christian Berentsen
  Details
g_object_unref_race_fix (1.30 KB, patch)
2009-01-09 09:20 UTC, Ole André Vadla Ravnås
rejected Details | Review

Description Jarl Christian Berentsen 2008-10-16 09:03:24 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.
Comment 1 Ole André Vadla Ravnås 2008-10-16 09:54:12 UTC
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]	
Comment 2 Jarl Christian Berentsen 2008-10-16 10:00:04 UTC
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);
Comment 3 Jarl Christian Berentsen 2008-10-16 14:13:41 UTC
CC otaylor as this was introduced with toggle_ref.
Comment 4 Ole André Vadla Ravnås 2009-01-09 09:20:55 UTC
Created attachment 126094 [details] [review]
g_object_unref_race_fix

Jarl Christian's suggested fix.
Comment 5 André Klapper 2009-02-18 21:02:56 UTC
can someone review the patch, please?
Comment 6 Antoine Tremblay 2009-07-07 19:27:53 UTC
This is a duplicate of bug #551706
Comment 7 Alberto Garcia 2009-09-23 08:07:42 UTC
Yes, this is a duplicate, both bugs even have very similar patches.
Comment 8 Tim Janik 2009-10-06 15:00:00 UTC
(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?
Comment 9 Cody Russell 2009-10-06 16:32:47 UTC

*** This bug has been marked as a duplicate of bug 551706 ***