Merge branch 'atomic-unref-cleanups' into 'main'

gobject: Use compare and exchange full to re-read old ref value

See merge request GNOME/glib!3098
This commit is contained in:
Philip Withnall 2022-12-06 16:12:59 +00:00
commit bbb3453c82
2 changed files with 412 additions and 17 deletions

View File

@ -3786,23 +3786,29 @@ g_object_unref (gpointer _object)
g_return_if_fail (G_IS_OBJECT (object));
/* here we want to atomically do: if (ref_count>1) { ref_count--; return; } */
retry_atomic_decrement1:
old_ref = g_atomic_int_get (&object->ref_count);
if (old_ref > 1)
retry_atomic_decrement1:
while (old_ref > 1)
{
/* valid if last 2 refs are owned by this call to unref and the toggle_ref */
gboolean has_toggle_ref = OBJECT_HAS_TOGGLE_REF (object);
if (!g_atomic_int_compare_and_exchange ((int *)&object->ref_count, old_ref, old_ref - 1))
goto retry_atomic_decrement1;
if (!g_atomic_int_compare_and_exchange_full ((int *)&object->ref_count,
old_ref, old_ref - 1,
&old_ref))
continue;
TRACE (GOBJECT_OBJECT_UNREF(object,G_TYPE_FROM_INSTANCE(object),old_ref));
/* if we went from 2->1 we need to notify toggle refs if any */
if (old_ref == 2 && has_toggle_ref) /* The last ref being held in this case is owned by the toggle_ref */
if (old_ref == 2 && OBJECT_HAS_TOGGLE_REF (object))
{
/* The last ref being held in this case is owned by the toggle_ref */
toggle_refs_notify (object, TRUE);
}
else
return;
}
{
GSList **weak_locations;
GObjectNotifyQueue *nqueue;
@ -3865,24 +3871,29 @@ g_object_unref (gpointer _object)
TRACE (GOBJECT_OBJECT_DISPOSE_END(object,G_TYPE_FROM_INSTANCE(object), 1));
/* may have been re-referenced meanwhile */
retry_atomic_decrement2:
old_ref = g_atomic_int_get ((int *)&object->ref_count);
if (old_ref > 1)
while (old_ref > 1)
{
/* valid if last 2 refs are owned by this call to unref and the toggle_ref */
gboolean has_toggle_ref = OBJECT_HAS_TOGGLE_REF (object);
if (!g_atomic_int_compare_and_exchange ((int *)&object->ref_count, old_ref, old_ref - 1))
goto retry_atomic_decrement2;
if (!g_atomic_int_compare_and_exchange_full ((int *)&object->ref_count,
old_ref, old_ref - 1,
&old_ref))
continue;
TRACE (GOBJECT_OBJECT_UNREF (object, G_TYPE_FROM_INSTANCE (object), old_ref));
/* emit all notifications that have been queued during dispose() */
g_object_notify_queue_thaw (object, nqueue);
TRACE (GOBJECT_OBJECT_UNREF(object,G_TYPE_FROM_INSTANCE(object),old_ref));
/* if we went from 2->1 we need to notify toggle refs if any */
if (old_ref == 2 && has_toggle_ref) /* The last ref being held in this case is owned by the toggle_ref */
if (old_ref == 2 && OBJECT_HAS_TOGGLE_REF (object) &&
g_atomic_int_get ((int *)&object->ref_count) == 1)
{
/* The last ref being held in this case is owned by the toggle_ref */
toggle_refs_notify (object, TRUE);
}
return;
}

View File

@ -731,6 +731,11 @@ toggle_notify (gpointer data,
g_assert (is_last == c->should_be_last);
if (is_last)
g_assert_cmpint (g_atomic_int_get (&obj->ref_count), ==, 1);
else
g_assert_cmpint (g_atomic_int_get (&obj->ref_count), ==, 2);
c->count++;
}
@ -779,6 +784,383 @@ test_toggle_ref (void)
g_object_remove_toggle_ref (obj, toggle_notify, &c);
}
G_DECLARE_FINAL_TYPE (DisposeReffingObject, dispose_reffing_object,
DISPOSE, REFFING_OBJECT, GObject)
typedef enum
{
PROP_INT_PROP = 1,
N_PROPS,
} DisposeReffingObjectProperty;
static GParamSpec *dispose_reffing_object_properties[N_PROPS] = {0};
struct _DisposeReffingObject
{
GObject parent;
GToggleNotify toggle_notify;
Count actual;
Count expected;
unsigned disposing_refs;
gboolean disposing_refs_all_normal;
GCallback notify_handler;
unsigned notify_called;
int int_prop;
GWeakRef *weak_ref;
};
G_DEFINE_TYPE (DisposeReffingObject, dispose_reffing_object, G_TYPE_OBJECT)
static void
on_object_notify (GObject *object,
GParamSpec *pspec,
void *data)
{
DisposeReffingObject *obj = DISPOSE_REFFING_OBJECT (object);
obj->notify_called++;
}
static void
dispose_reffing_object_dispose (GObject *object)
{
DisposeReffingObject *obj = DISPOSE_REFFING_OBJECT (object);
g_assert_cmpint (object->ref_count, ==, 1);
g_assert_cmpint (obj->actual.count, ==, obj->expected.count);
for (unsigned i = 0; i < obj->disposing_refs; ++i)
{
if (i == 0 && !obj->disposing_refs_all_normal)
{
g_object_add_toggle_ref (object, obj->toggle_notify, &obj->actual);
}
else
{
obj->actual.should_be_last = FALSE;
g_object_ref (obj);
g_assert_cmpint (obj->actual.count, ==, obj->expected.count);
}
obj->actual.should_be_last = TRUE;
}
G_OBJECT_CLASS (dispose_reffing_object_parent_class)->dispose (object);
if (obj->notify_handler)
{
unsigned old_notify_called = obj->notify_called;
g_assert_cmpuint (g_signal_handler_find (object, G_SIGNAL_MATCH_FUNC,
0, 0, NULL, obj->notify_handler, NULL), ==, 0);
g_signal_connect (object, "notify", G_CALLBACK (obj->notify_handler), NULL);
/* This would trigger a toggle notification, but is not something we may
* want with https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2377
* so, we only test this in case we have more than one ref
*/
if (obj->toggle_notify == toggle_notify)
g_assert_cmpint (obj->disposing_refs, >, 1);
g_object_notify (object, "int-prop");
g_assert_cmpuint (obj->notify_called, ==, old_notify_called);
}
g_assert_cmpint (obj->actual.count, ==, obj->expected.count);
}
static void
dispose_reffing_object_init (DisposeReffingObject *connector)
{
}
static void
dispose_reffing_object_set_property (GObject *object,
guint property_id,
const GValue *value,
GParamSpec *pspec)
{
DisposeReffingObject *obj = DISPOSE_REFFING_OBJECT (object);
switch ((DisposeReffingObjectProperty) property_id)
{
case PROP_INT_PROP:
obj->int_prop = g_value_get_int (value);
break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
break;
}
}
static void
dispose_reffing_object_get_property (GObject *object,
guint property_id,
GValue *value,
GParamSpec *pspec)
{
DisposeReffingObject *obj = DISPOSE_REFFING_OBJECT (object);
switch ((DisposeReffingObjectProperty) property_id)
{
case PROP_INT_PROP:
g_value_set_int (value, obj->int_prop);
break;
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
break;
}
}
static void
dispose_reffing_object_class_init (DisposeReffingObjectClass *klass)
{
GObjectClass *object_class = G_OBJECT_CLASS (klass);
dispose_reffing_object_properties[PROP_INT_PROP] =
g_param_spec_int ("int-prop", "int-prop", "int-prop",
G_MININT, G_MAXINT,
0,
G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS);
object_class->dispose = dispose_reffing_object_dispose;
object_class->set_property = dispose_reffing_object_set_property;
object_class->get_property = dispose_reffing_object_get_property;
g_object_class_install_properties (object_class, N_PROPS,
dispose_reffing_object_properties);
}
static void
test_toggle_ref_on_dispose (void)
{
DisposeReffingObject *obj;
gpointer disposed_checker = &obj;
/* This tests wants to ensure that an object that gets re-referenced
* (one or multiple times) during its dispose virtual function:
* - Notifies all the queued "notify" signal handlers
* - Notifies toggle notifications if any
* - It does not get finalized
*/
obj = g_object_new (dispose_reffing_object_get_type (), NULL);
obj->toggle_notify = toggle_notify;
obj->notify_handler = G_CALLBACK (on_object_notify);
g_object_add_weak_pointer (G_OBJECT (obj), &disposed_checker);
/* Convert to toggle notification */
g_object_add_toggle_ref (G_OBJECT (obj), obj->toggle_notify, &obj->actual);
g_assert_cmpint (obj->actual.count, ==, 0);
obj->actual.should_be_last = TRUE;
obj->notify_handler = G_CALLBACK (on_object_notify);
g_object_unref (obj);
g_assert_cmpint (obj->actual.count, ==, 1);
g_assert_cmpuint (obj->notify_called, ==, 0);
/* Remove the toggle reference, making it to dispose and resurrect again */
obj->disposing_refs = 1;
obj->expected.count = 1;
obj->notify_handler = NULL; /* FIXME: enable it when !2377 is in */
g_object_remove_toggle_ref (G_OBJECT (obj), obj->toggle_notify, NULL);
g_assert_cmpint (obj->actual.count, ==, 2);
g_assert_cmpuint (obj->notify_called, ==, 0);
g_assert_null (disposed_checker);
g_assert_cmpint (g_atomic_int_get (&G_OBJECT (obj)->ref_count), ==,
obj->disposing_refs);
/* Object has been disposed, but is still alive, so add another weak pointer */
disposed_checker = &obj;
g_object_add_weak_pointer (G_OBJECT (obj), &disposed_checker);
/* Remove the toggle reference, making it to dispose and resurrect with
* more references than before, so that no toggle notify is called
*/
obj->disposing_refs = 3;
obj->expected.count = 2;
obj->notify_handler = G_CALLBACK (on_object_notify);
g_object_remove_toggle_ref (G_OBJECT (obj), obj->toggle_notify, NULL);
g_assert_cmpint (obj->actual.count, ==, 2);
g_assert_cmpint (obj->notify_called, ==, 1);
obj->expected.count = obj->actual.count;
g_assert_null (disposed_checker);
g_assert_cmpint (g_atomic_int_get (&G_OBJECT (obj)->ref_count), ==,
obj->disposing_refs);
disposed_checker = &obj;
g_object_add_weak_pointer (G_OBJECT (obj), &disposed_checker);
/* Now remove the first added reference */
obj->disposing_refs = 0;
g_object_unref (obj);
g_assert_nonnull (disposed_checker);
g_assert_cmpint (g_atomic_int_get (&G_OBJECT (obj)->ref_count), ==, 2);
g_assert_cmpint (obj->actual.count, ==, 2);
g_assert_cmpint (obj->notify_called, ==, 1);
/* And the toggle one */
obj->actual.should_be_last = TRUE;
obj->notify_handler = NULL;
g_object_remove_toggle_ref (G_OBJECT (obj), obj->toggle_notify, NULL);
g_assert_nonnull (disposed_checker);
g_assert_cmpint (g_atomic_int_get (&G_OBJECT (obj)->ref_count), ==, 1);
g_assert_cmpint (obj->actual.count, ==, 2);
obj->expected.count = obj->actual.count;
g_clear_object (&obj);
g_assert_null (disposed_checker);
}
static void
toggle_notify_counter (gpointer data,
GObject *obj,
gboolean is_last)
{
Count *c = data;
c->count++;
if (is_last)
g_assert_cmpint (g_atomic_int_get (&obj->ref_count), ==, 1);
else
g_assert_cmpint (g_atomic_int_get (&obj->ref_count), ==, 2);
}
static void
on_object_notify_switch_to_normal_ref (GObject *object,
GParamSpec *pspec,
void *data)
{
DisposeReffingObject *obj = DISPOSE_REFFING_OBJECT (object);
obj->notify_called++;
g_object_ref (object);
g_object_remove_toggle_ref (object, obj->toggle_notify, NULL);
}
static void
on_object_notify_switch_to_toggle_ref (GObject *object,
GParamSpec *pspec,
void *data)
{
DisposeReffingObject *obj = DISPOSE_REFFING_OBJECT (object);
obj->notify_called++;
g_object_add_toggle_ref (object, obj->toggle_notify, &obj->actual);
g_object_unref (object);
}
static void
on_object_notify_add_ref (GObject *object,
GParamSpec *pspec,
void *data)
{
DisposeReffingObject *obj = DISPOSE_REFFING_OBJECT (object);
int old_toggle_cout = obj->actual.count;
obj->notify_called++;
g_object_ref (object);
g_assert_cmpint (obj->actual.count, ==, old_toggle_cout);
}
static void
test_toggle_ref_and_notify_on_dispose (void)
{
DisposeReffingObject *obj;
gpointer disposed_checker = &obj;
/* This tests wants to ensure that toggle signal emission during dispose
* is properly working if the object is revitalized by adding new references.
* It also wants to check that toggle notifications are not happening if a
* notify handler is removing them at this phase.
*/
obj = g_object_new (dispose_reffing_object_get_type (), NULL);
obj->toggle_notify = toggle_notify_counter;
g_object_add_weak_pointer (G_OBJECT (obj), &disposed_checker);
/* Convert to toggle notification */
g_object_add_toggle_ref (G_OBJECT (obj), obj->toggle_notify, &obj->actual);
g_assert_cmpint (obj->actual.count, ==, 0);
obj->notify_handler = G_CALLBACK (on_object_notify);
g_object_unref (obj);
g_assert_cmpint (obj->actual.count, ==, 1);
g_assert_cmpuint (obj->notify_called, ==, 0);
disposed_checker = &obj;
g_object_add_weak_pointer (G_OBJECT (obj), &disposed_checker);
/* Check that notification is triggered after being queued */
obj->disposing_refs = 1;
obj->expected.count = 1;
obj->notify_handler = G_CALLBACK (on_object_notify);
g_object_remove_toggle_ref (G_OBJECT (obj), obj->toggle_notify, NULL);
/* FIXME: adjust the count to 1 when !2377 is in */
g_assert_cmpint (obj->actual.count, ==, 4);
g_assert_cmpuint (obj->notify_called, ==, 1);
disposed_checker = &obj;
g_object_add_weak_pointer (G_OBJECT (obj), &disposed_checker);
/* Check that notification is triggered after being queued, but no toggle
* notification is happening if notify handler switches to normal reference
*/
obj->disposing_refs = 1;
obj->expected.count = 4;
obj->notify_handler = G_CALLBACK (on_object_notify_switch_to_normal_ref);
g_object_remove_toggle_ref (G_OBJECT (obj), obj->toggle_notify, NULL);
g_assert_cmpint (obj->actual.count, ==, 5);
g_assert_cmpuint (obj->notify_called, ==, 2);
disposed_checker = &obj;
g_object_add_weak_pointer (G_OBJECT (obj), &disposed_checker);
/* Check that notification is triggered after being queued, but that toggle
* is happening if notify handler switched to toggle reference
*/
obj->disposing_refs = 1;
obj->disposing_refs_all_normal = TRUE;
obj->expected.count = 5;
obj->notify_handler = G_CALLBACK (on_object_notify_switch_to_toggle_ref);
g_object_unref (obj);
g_assert_cmpint (obj->actual.count, ==, 7);
g_assert_cmpuint (obj->notify_called, ==, 3);
disposed_checker = &obj;
g_object_add_weak_pointer (G_OBJECT (obj), &disposed_checker);
/* Check that notification is triggered after being queued, but that toggle
* is not happening if current refcount changed.
*/
obj->disposing_refs = 1;
obj->disposing_refs_all_normal = FALSE;
obj->expected.count = 7;
obj->notify_handler = G_CALLBACK (on_object_notify_add_ref);
g_object_remove_toggle_ref (G_OBJECT (obj), obj->toggle_notify, NULL);
g_assert_cmpint (obj->actual.count, ==, 8);
g_assert_cmpuint (obj->notify_called, ==, 4);
g_object_unref (obj);
disposed_checker = &obj;
g_object_add_weak_pointer (G_OBJECT (obj), &disposed_checker);
obj->disposing_refs = 0;
obj->expected.count = 9;
g_clear_object (&obj);
g_assert_null (disposed_checker);
}
static gboolean global_destroyed;
static gint global_value;
@ -982,6 +1364,8 @@ main (int argc, char **argv)
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/toggle-ref/ref-on-dispose", test_toggle_ref_on_dispose);
g_test_add_func ("/object/toggle-ref/ref-and-notify-on-dispose", test_toggle_ref_and_notify_on_dispose);
g_test_add_func ("/object/qdata", test_object_qdata);
g_test_add_func ("/object/qdata2", test_object_qdata2);