gobject: Separate GWeakRef from GWeakNotify

This patch is based upon Garrett Regier's work from 2015 to provide
some reliability and predictability to how disposal handles weak
reference state.

A primary problem is that GWeakRef and GWeakNotify state is shared and
therefore you cannot rely on GWeakRef status due to a GWeakNotify
calling into second-degree code.

It's important to ensure that both weak pointer locations and GWeakRef
will do the proper thing before user callbacks are executed during
disposal. Otherwise, user callbacks cannot rely on the status of their
weak pointers. That would be mostly okay but becomes an issue when
second degree objects are then disposed before any notification of
dependent object disposal.

Consider objects A and B.

`A` contains a reference to `B` and `B` contains a `GWeakRef` to `A`.
When `A` is disposed, `B` may be disposed as a consequence but has not
yet been notified that `A` has been disposed. It's `GWeakRef` may also
cause liveness issues if `GWeakNotify` on `A` result in tertiary code
running which wants to interact with `B`.

This example is analagous to how `GtkTextView` and `GtkTextBuffer` work
in text editing applications.

To provide application and libraries the ability to handle this using
already existing API, `GWeakRef` is separated into it's own GData quark
so that weak locations and `GWeakRef` are cleared before user code is
executed as a consequence of `GData` cleanup.

# Conflicts:
#	gobject/tests/signals.c
This commit is contained in:
Christian Hergert 2023-08-25 14:20:27 -07:00
parent fd2d1e829e
commit eb8a33625e
2 changed files with 53 additions and 4 deletions

View File

@ -260,6 +260,7 @@ G_LOCK_DEFINE_STATIC (weak_refs_mutex);
G_LOCK_DEFINE_STATIC (toggle_refs_mutex);
static GQuark quark_closure_array = 0;
static GQuark quark_weak_refs = 0;
static GQuark quark_weak_notifies = 0;
static GQuark quark_toggle_refs = 0;
static GQuark quark_notify_queue;
#ifndef HAVE_OPTIONAL_FLAGS
@ -523,6 +524,7 @@ g_object_do_class_init (GObjectClass *class)
quark_closure_array = g_quark_from_static_string ("GObject-closure-array");
quark_weak_refs = g_quark_from_static_string ("GObject-weak-references");
quark_weak_notifies = g_quark_from_static_string ("GObject-weak-notifies");
quark_weak_locations = g_quark_from_static_string ("GObject-weak-locations");
quark_toggle_refs = g_quark_from_static_string ("GObject-toggle-references");
quark_notify_queue = g_quark_from_static_string ("GObject-notify-queue");
@ -1359,9 +1361,16 @@ static void
g_object_real_dispose (GObject *object)
{
g_signal_handlers_destroy (object);
g_datalist_id_set_data (&object->qdata, quark_closure_array, NULL);
/* GWeakRef and weak_pointer do not call into user code. Clear those first
* so that user code can rely on the state of their weak pointers.
*/
g_datalist_id_set_data (&object->qdata, quark_weak_refs, NULL);
g_datalist_id_set_data (&object->qdata, quark_weak_locations, NULL);
/* GWeakNotify and GClosure can call into user code */
g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);
g_datalist_id_set_data (&object->qdata, quark_closure_array, NULL);
}
#ifdef G_ENABLE_DEBUG
@ -3316,7 +3325,7 @@ g_object_weak_ref (GObject *object,
g_return_if_fail (g_atomic_int_get (&object->ref_count) >= 1);
G_LOCK (weak_refs_mutex);
wstack = g_datalist_id_remove_no_notify (&object->qdata, quark_weak_refs);
wstack = g_datalist_id_remove_no_notify (&object->qdata, quark_weak_notifies);
if (wstack)
{
i = wstack->n_weak_refs++;
@ -3331,7 +3340,7 @@ g_object_weak_ref (GObject *object,
}
wstack->weak_refs[i].notify = notify;
wstack->weak_refs[i].data = data;
g_datalist_id_set_data_full (&object->qdata, quark_weak_refs, wstack, weak_refs_notify);
g_datalist_id_set_data_full (&object->qdata, quark_weak_notifies, wstack, weak_refs_notify);
G_UNLOCK (weak_refs_mutex);
}
@ -3355,7 +3364,7 @@ g_object_weak_unref (GObject *object,
g_return_if_fail (notify != NULL);
G_LOCK (weak_refs_mutex);
wstack = g_datalist_id_get_data (&object->qdata, quark_weak_refs);
wstack = g_datalist_id_get_data (&object->qdata, quark_weak_notifies);
if (wstack)
{
guint i;
@ -3927,6 +3936,7 @@ g_object_unref (gpointer _object)
g_signal_handlers_destroy (object);
g_datalist_id_set_data (&object->qdata, quark_weak_refs, NULL);
g_datalist_id_set_data (&object->qdata, quark_weak_locations, NULL);
g_datalist_id_set_data (&object->qdata, quark_weak_notifies, NULL);
/* decrement the last reference */
old_ref = g_atomic_int_add (&object->ref_count, -1);

View File

@ -2045,6 +2045,44 @@ test_emitv (void)
g_array_unref (values);
}
typedef struct
{
GWeakRef wr;
gulong handler;
} TestWeakRefDisconnect;
static void
weak_ref_disconnect_notify (gpointer data,
GObject *where_object_was)
{
TestWeakRefDisconnect *state = data;
g_assert_null (g_weak_ref_get (&state->wr));
state->handler = 0;
}
static void
test_weak_ref_disconnect (void)
{
TestWeakRefDisconnect state;
GObject *test;
test = g_object_new (test_get_type (), NULL);
g_weak_ref_init (&state.wr, test);
state.handler = g_signal_connect_data (test,
"simple",
G_CALLBACK (dont_reach),
&state,
(GClosureNotify) weak_ref_disconnect_notify,
0);
g_assert_cmpint (state.handler, >, 0);
g_object_unref (test);
g_assert_cmpint (state.handler, ==, 0);
g_assert_null (g_weak_ref_get (&state.wr));
g_weak_ref_clear (&state.wr);
}
/* --- */
int
@ -2083,6 +2121,7 @@ main (int argc,
g_test_add_data_func ("/gobject/signals/invalid-name/first-char", "7zip", test_signals_invalid_name);
g_test_add_data_func ("/gobject/signals/invalid-name/empty", "", test_signals_invalid_name);
g_test_add_func ("/gobject/signals/is-valid-name", test_signal_is_valid_name);
g_test_add_func ("/gobject/signals/weak-ref-disconnect", test_weak_ref_disconnect);
return g_test_run ();
}