luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2024-11-10 11:26:16 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'always-cleanup-weak-locations' into 'main'

Browse Source 
gobject: Cleanup GWeakRef locations on object finalization

Closes #2390 and #865

See merge request GNOME/glib!2064
This commit is contained in:
Philip Withnall 2021-09-20 12:24:47 +00:00
commit 3e57fc4c02
3 changed files with 269 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
gobject/gobject.c
View File

@ -233,6 +233,7 @@ static guint object_floating_flag_handler (GObject *object,
static void object_interface_check_properties (gpointer check_data,
gpointer g_iface);
static void weak_locations_free_unlocked (GSList **weak_locations);
/* --- typedefs --- */
typedef struct _GObjectNotifyQueue GObjectNotifyQueue;
@ -1178,6 +1179,7 @@ g_object_real_dispose (GObject *object)
g_signal_handlers_destroy (object);
g_datalist_id_set_data (&object->qdata, quark_closure_array, NULL);
g_datalist_id_set_data (&object->qdata, quark_weak_refs, NULL);
g_datalist_id_set_data (&object->qdata, quark_weak_locations, NULL);
}
static void
@ -3512,6 +3514,10 @@ g_object_unref (gpointer _object)
* established at this time, because the other thread would have
* to hold a strong ref in order to call
* g_object_add_weak_pointer() and then we wouldn't be here.
*
* Other GWeakRef's (weak locations) instead may still be added
* before the object is finalized, but in such case we'll unset
* them as part of the qdata removal.
*/
weak_locations = g_datalist_id_get_data (&object->qdata, quark_weak_locations);
@ -3531,15 +3537,11 @@ g_object_unref (gpointer _object)
}
/* We got the lock first, so the object will definitely die
* now. Clear out all the weak references.
* now. Clear out all the weak references, if they're still set.
*/
while (*weak_locations)
{
GWeakRef *weak_ref_location = (*weak_locations)->data;
weak_ref_location->priv.p = NULL;
*weak_locations = g_slist_delete_link (*weak_locations, *weak_locations);
}
weak_locations = g_datalist_id_remove_no_notify (&object->qdata,
quark_weak_locations);
g_clear_pointer (&weak_locations, weak_locations_free_unlocked);
g_rw_lock_writer_unlock (&weak_locations_lock);
}
@ -3573,7 +3575,8 @@ g_object_unref (gpointer _object)
g_datalist_id_set_data (&object->qdata, quark_closure_array, NULL);
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);
/* decrement the last reference */
old_ref = g_atomic_int_add (&object->ref_count, -1);
g_return_if_fail (old_ref > 0);
@ -4646,6 +4649,35 @@ g_weak_ref_get (GWeakRef *weak_ref)
return object_or_null;
}
static void
weak_locations_free_unlocked (GSList **weak_locations)
{
if (*weak_locations)
{
GSList *weak_location;
for (weak_location = *weak_locations; weak_location;)
{
GWeakRef *weak_ref_location = weak_location->data;
weak_ref_location->priv.p = NULL;
weak_location = g_slist_delete_link (weak_location, weak_location);
}
}
g_free (weak_locations);
}
static void
weak_locations_free (gpointer data)
{
GSList **weak_locations = data;
g_rw_lock_writer_lock (&weak_locations_lock);
weak_locations_free_unlocked (weak_locations);
g_rw_lock_writer_unlock (&weak_locations_lock);
}
/**
* g_weak_ref_set: (skip)
* @weak_ref: location for a weak reference
@ -4702,6 +4734,12 @@ g_weak_ref_set (GWeakRef *weak_ref,
g_assert (weak_locations != NULL);
*weak_locations = g_slist_remove (*weak_locations, weak_ref);
if (!*weak_locations)
{
weak_locations_free_unlocked (weak_locations);
g_datalist_id_remove_no_notify (&old_object->qdata, quark_weak_locations);
}
}
/* Add the weak ref to the new object */
@ -4712,7 +4750,8 @@ g_weak_ref_set (GWeakRef *weak_ref,
if (weak_locations == NULL)
{
weak_locations = g_new0 (GSList *, 1);
g_datalist_id_set_data_full (&new_object->qdata, quark_weak_locations, weak_locations, g_free);
g_datalist_id_set_data_full (&new_object->qdata, quark_weak_locations,
weak_locations, weak_locations_free);
}
*weak_locations = g_slist_prepend (*weak_locations, weak_ref);

139
gobject/tests/reference.c
View File

@ -558,6 +558,18 @@ test_weak_ref (void)
g_weak_ref_clear (&weak3);
/* unset dynamic_weak... */
g_weak_ref_set (dynamic_weak, NULL);
g_assert_null (g_weak_ref_get (dynamic_weak));
/* initializing a weak reference to an object that had before works */
g_weak_ref_set (dynamic_weak, obj2);
tmp = g_weak_ref_get (dynamic_weak);
g_assert_true (tmp == obj2);
g_assert_cmpint (obj2->ref_count, ==, 2);
g_object_unref (tmp);
g_assert_cmpint (obj2->ref_count, ==, 1);
/* clear and free dynamic_weak... */
g_weak_ref_clear (dynamic_weak);
@ -566,6 +578,130 @@ test_weak_ref (void)
g_free (dynamic_weak);
}
G_DECLARE_FINAL_TYPE (WeakReffedObject, weak_reffed_object,
WEAK, REFFED_OBJECT, GObject)
struct _WeakReffedObject
{
GObject parent;
GWeakRef *weak_ref;
};
G_DEFINE_TYPE (WeakReffedObject, weak_reffed_object, G_TYPE_OBJECT)
static void
weak_reffed_object_dispose (GObject *object)
{
WeakReffedObject *weak_reffed = WEAK_REFFED_OBJECT (object);
g_assert_cmpint (object->ref_count, ==, 1);
g_weak_ref_set (weak_reffed->weak_ref, object);
G_OBJECT_CLASS (weak_reffed_object_parent_class)->dispose (object);
g_assert_null (g_weak_ref_get (weak_reffed->weak_ref));
}
static void
weak_reffed_object_init (WeakReffedObject *connector)
{
}
static void
weak_reffed_object_class_init (WeakReffedObjectClass *klass)
{
GObjectClass *object_class = G_OBJECT_CLASS (klass);
object_class->dispose = weak_reffed_object_dispose;
}
static void
test_weak_ref_on_dispose (void)
{
WeakReffedObject *obj;
GWeakRef weak = { { GUINT_TO_POINTER (0xDEADBEEFU) } };
g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2390");
g_test_summary ("Test that a weak ref set during dispose vfunc is cleared");
g_weak_ref_init (&weak, NULL);
obj = g_object_new (weak_reffed_object_get_type (), NULL);
obj->weak_ref = &weak;
g_assert_cmpint (G_OBJECT (obj)->ref_count, ==, 1);
g_clear_object (&obj);
g_assert_null (g_weak_ref_get (&weak));
}
static void
test_weak_ref_on_run_dispose (void)
{
GObject *obj;
GWeakRef weak = { { GUINT_TO_POINTER (0xDEADBEEFU) } };
g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/865");
g_test_summary ("Test that a weak ref is cleared on g_object_run_dispose()");
obj = g_object_new (G_TYPE_OBJECT, NULL);
g_weak_ref_init (&weak, obj);
g_assert_true (obj == g_weak_ref_get (&weak));
g_object_unref (obj);
g_object_run_dispose (obj);
g_assert_null (g_weak_ref_get (&weak));
g_clear_object (&obj);
g_assert_null (g_weak_ref_get (&weak));
}
static void
on_weak_ref_toggle_notify (gpointer data,
GObject *object,
gboolean is_last_ref)
{
GWeakRef *weak = data;
if (is_last_ref)
g_weak_ref_set (weak, object);
}
static void
on_weak_ref_toggle_notify_disposed (gpointer data,
GObject *object)
{
g_assert_cmpint (object->ref_count, ==, 1);
g_object_ref (object);
g_object_unref (object);
}
static void
test_weak_ref_on_toggle_notify (void)
{
GObject *obj;
GWeakRef weak = { { GUINT_TO_POINTER (0xDEADBEEFU) } };
g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2390");
g_test_summary ("Test that a weak ref set on toggle notify is cleared");
g_weak_ref_init (&weak, NULL);
obj = g_object_new (G_TYPE_OBJECT, NULL);
g_object_add_toggle_ref (obj, on_weak_ref_toggle_notify, &weak);
g_object_weak_ref (obj, on_weak_ref_toggle_notify_disposed, NULL);
g_object_unref (obj);
g_assert_cmpint (obj->ref_count, ==, 1);
g_clear_object (&obj);
g_assert_null (g_weak_ref_get (&weak));
}
typedef struct
{
gboolean should_be_last;
@ -826,6 +962,9 @@ main (int argc, char **argv)
g_test_add_func ("/object/weak-pointer/set", test_weak_pointer_set);
g_test_add_func ("/object/weak-pointer/set-function", test_weak_pointer_set_function);
g_test_add_func ("/object/weak-ref", test_weak_ref);
g_test_add_func ("/object/weak-ref/on-dispose", test_weak_ref_on_dispose);
g_test_add_func ("/object/weak-ref/on-run-dispose", test_weak_ref_on_run_dispose);
g_test_add_func ("/object/weak-ref/on-toggle-notify", test_weak_ref_on_toggle_notify);
g_test_add_func ("/object/toggle-ref", test_toggle_ref);
g_test_add_func ("/object/qdata", test_object_qdata);
g_test_add_func ("/object/qdata2", test_object_qdata2);

81
gobject/tests/threadtests.c
View File

@ -343,6 +343,85 @@ test_threaded_weak_ref (void)
get_wins, unref_wins);
}
typedef struct
{
GObject *object;
GWeakRef *weak;
gint started; /* (atomic) */
gint finished; /* (atomic) */
gint disposing; /* (atomic) */
} ThreadedWeakRefData;
static void
on_weak_ref_disposed (gpointer data,
GObject *gobj)
{
ThreadedWeakRefData *thread_data = data;
/* Wait until the thread has started */
while (!g_atomic_int_get (&thread_data->started))
continue;
g_atomic_int_set (&thread_data->disposing, 1);
/* Wait for the thread to act, so that the object is still valid */
while (!g_atomic_int_get (&thread_data->finished))
continue;
g_atomic_int_set (&thread_data->disposing, 0);
}
static gpointer
on_other_thread_weak_ref (gpointer user_data)
{
ThreadedWeakRefData *thread_data = user_data;
GObject *object = thread_data->object;
g_atomic_int_set (&thread_data->started, 1);
/* Ensure we've started disposal */
while (!g_atomic_int_get (&thread_data->disposing))
continue;
g_object_ref (object);
g_weak_ref_set (thread_data->weak, object);
g_object_unref (object);
g_assert_cmpint (thread_data->disposing, ==, 1);
g_atomic_int_set (&thread_data->finished, 1);
return NULL;
}
static void
test_threaded_weak_ref_finalization (void)
{
GObject *obj = g_object_new (G_TYPE_OBJECT, NULL);
GWeakRef weak = { { GUINT_TO_POINTER (0xDEADBEEFU) } };
ThreadedWeakRefData thread_data = {
.object = obj, .weak = &weak, .started = 0, .finished = 0
};
g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2390");
g_test_summary ("Test that a weak ref added by another thread during dispose "
"of a GObject is cleared during finalisation. "
"Use on_weak_ref_disposed() to synchronize the other thread "
"with the dispose vfunc.");
g_weak_ref_init (&weak, NULL);
g_object_weak_ref (obj, on_weak_ref_disposed, &thread_data);
g_assert_cmpint (obj->ref_count, ==, 1);
g_thread_unref (g_thread_new ("on_other_thread",
on_other_thread_weak_ref,
&thread_data));
g_object_unref (obj);
/* This is what this test is about: at this point the weak reference
* should have been unset (and not point to a dead object either). */
g_assert_null (g_weak_ref_get (&weak));
}
int
main (int argc,
char *argv[])
@ -352,6 +431,8 @@ main (int argc,
/* g_test_add_func ("/GObject/threaded-class-init", test_threaded_class_init); */
g_test_add_func ("/GObject/threaded-object-init", test_threaded_object_init);
g_test_add_func ("/GObject/threaded-weak-ref", test_threaded_weak_ref);
g_test_add_func ("/GObject/threaded-weak-ref/on-finalization",
test_threaded_weak_ref_finalization);
return g_test_run();
}