Deadlock when using GWeakRef's on toggled notification
In Gjs we're starting to use GWeakRef
to keep track of objects references during a toggle notification, as those normally happens as per other threads ref/unref events.
However, I was hoping to be able to use them both in case we get a toggled reference and when we get an untoggled one, but in first case things aren't working as expected.
I know that the documentation mentions using strong references in such case, but I preferred to reduce the ref/unref events from our side that may cause further toggle events to happen.
By doing that however we may end up in a pretty clear starvation, here's a small reproducer:
#include <glib.h>
#include <gio/gio.h>
static GWeakRef weak_ref;
static gboolean toggle_notifying = FALSE;
static void
on_toggle_notify (gpointer data, GObject* gobj, gboolean is_last_ref)
{
g_print("Got toggle notify for %p (ref %d), last ref: %d, recursion: %d\n",
gobj, gobj->ref_count, is_last_ref, toggle_notifying);
if (!is_last_ref)
g_print("Creating another weak reference... Ready to dead-lock!?\n");
toggle_notifying = TRUE;
GWeakRef other_weak_ref;
g_weak_ref_init(&other_weak_ref, NULL);
toggle_notifying = FALSE;
}
void
main(void)
{
GFile *file = g_file_new_for_path("/");
g_object_add_toggle_ref(G_OBJECT(file), on_toggle_notify, NULL);
g_object_unref (file);
g_weak_ref_set (&weak_ref, file);
g_print("Object references... %d, getting the weak one\n", G_OBJECT(file)->ref_count);
g_object_unref (g_weak_ref_get(&weak_ref));
g_clear_object (&file);
}
Output
Got toggle notify for 0x55555558ee00 (ref 1), last ref: 1, recursion: 0
Object references... 1, getting the weak one
Got toggle notify for 0x55555558ee00 (ref 2), last ref: 0, recursion: 0
Creating another reference... Ready to dead-lock!?
#0 __futex_abstimed_wait_common64 (cancel=false, private=<optimized out>, abstime=0x0, clockid=0, expected=2,
futex_word=0x55555558fd78) at ../sysdeps/nptl/futex-internal.c:87
#1 __GI___futex_abstimed_wait64 (futex_word=futex_word@entry=0x55555558fd78, expected=expected@entry=2,
clockid=clockid@entry=0, abstime=abstime@entry=0x0, private=<optimized out>)
at ../sysdeps/nptl/futex-internal.c:112
#2 0x00007ffff793c3b7 in __pthread_rwlock_wrlock_full64 (abstime=0x0, clockid=0, rwlock=0x55555558fd70)
at pthread_rwlock_common.c:829
#3 __GI___pthread_rwlock_wrlock (rwlock=0x55555558fd70) at pthread_rwlock_wrlock.c:27
#4 0x00007ffff7cef7c5 in g_rw_lock_writer_lock (rw_lock=rw_lock@entry=0x7ffff7dd85f0 <weak_locations_lock>)
at ../../glib/glib/gthread-posix.c:476
#5 0x00007ffff7d9cb7d in g_weak_ref_set (weak_ref=0x555555558018 <weak_ref>, object=0x55555558ee00)
at ../../glib/gobject/gobject.c:4611
#6 0x000055555555529f in on_toggle_notify ()
#7 0x00007ffff7d96eb7 in g_object_ref (_object=_object@entry=0x55555558ee00) at ../../glib/gobject/gobject.c:3396
#8 0x00007ffff7d9caef in g_weak_ref_get (weak_ref=0x555555558018 <weak_ref>) at ../../glib/gobject/gobject.c:4578
#9 0x000055555555534e in main ()
Adding some locking debug to GObject:
g_rw_lock_writer_lock (&weak_locations_lock); g_weak_ref_set ../../glib/gobject/gobject.c:4634
g_rw_lock_writer_unlock (&weak_locations_lock); g_weak_ref_set ../../glib/gobject/gobject.c:4682
Got toggle notify for 0x55abe31b9e00 (ref 1), last ref: 1, recursion: 0
g_rw_lock_writer_lock (&weak_locations_lock); g_weak_ref_set ../../glib/gobject/gobject.c:4634
g_rw_lock_writer_unlock (&weak_locations_lock); g_weak_ref_set ../../glib/gobject/gobject.c:4682
g_rw_lock_writer_lock (&weak_locations_lock); g_weak_ref_set ../../glib/gobject/gobject.c:4634
g_rw_lock_writer_unlock (&weak_locations_lock); g_weak_ref_set ../../glib/gobject/gobject.c:4682
Object references... 1, getting the weak one
g_rw_lock_reader_lock (&weak_locations_lock); g_weak_ref_get ../../glib/gobject/gobject.c:4594
Got toggle notify for 0x55abe31b9e00 (ref 2), last ref: 0, recursion: 0
Creating another weak reference... Ready to dead-lock!?
g_rw_lock_writer_lock (&weak_locations_lock); g_weak_ref_set ../../glib/gobject/gobject.c:4634
Now... As it was quite clear already by calling g_weak_ref_get
we add a reference, that makes the toggle notify to happen, now in this callback we want to create another weak reference, but we can't because we're locked as per the previous lock that has been set when g_weak_ref_get
itself was called.
So, I think that to solve this in toggle_refs_notify
we could just temporary release the lock if is_last_ref
is FALSE
while calling the notify callback, and it seems sane as the lock we've set in g_weak_ref_get
was to prevent that the object might be unreferenced again before we've referenced it again, but at the point we call the notify callback this should not be a problem anymore as we've just referenced...
Anyways, any idea?
/cc @smcv as you were one of the original authors as per commit 46c2f570