diff --git a/glib/gdataset.c b/glib/gdataset.c index 6b78d2e33..500022630 100644 --- a/glib/gdataset.c +++ b/glib/gdataset.c @@ -504,7 +504,12 @@ g_data_remove_internal (GData **datalist, GDataElt *old, *data, *data_end; gsize found_keys; - old = g_newa (GDataElt, n_keys); + /* Allocate an array of GDataElt to hold copies of the elements + * that are removed from the datalist. Allow enough space for all + * the keys; if a key is not found, the corresponding element of + * old is not populated, so we initialize them all to NULL to + * detect that case. */ + old = g_newa0 (GDataElt, n_keys); data = d->data; data_end = data + d->len; @@ -518,6 +523,7 @@ g_data_remove_internal (GData **datalist, { if (data->key == keys[i]) { + old[i] = *data; remove = TRUE; break; } @@ -525,15 +531,14 @@ g_data_remove_internal (GData **datalist, if (remove) { - old[found_keys] = *data; + GDataElt *data_last = data_end - 1; + found_keys++; - if (data < data_end) - { - data_end--; - *data = *data_end; - } + if (data < data_last) + *data = *data_last; + data_end--; d->len--; /* We don't bother to shrink, but if all data are now gone @@ -546,16 +551,21 @@ g_data_remove_internal (GData **datalist, break; } } - - data++; + else + { + data++; + } } if (found_keys > 0) { g_datalist_unlock (datalist); - for (gsize i = 0; i < found_keys; i++) + for (gsize i = 0; i < n_keys; i++) { + /* If keys[i] was not found, then old[i].destroy is NULL. + * Call old[i].destroy() only if keys[i] was found, and + * is associated with a destroy notifier: */ if (old[i].destroy) old[i].destroy (old[i].data); } diff --git a/glib/tests/dataset.c b/glib/tests/dataset.c index 3b96b42a7..7541268d6 100644 --- a/glib/tests/dataset.c +++ b/glib/tests/dataset.c @@ -250,6 +250,75 @@ test_datalist_id (void) g_datalist_clear (&list); } +static void +test_datalist_id_remove_multiple (void) +{ + /* Test that g_datalist_id_remove_multiple() removes all the keys it + * is given. */ + GData *list = NULL; + GQuark one = g_quark_from_static_string ("one"); + GQuark two = g_quark_from_static_string ("two"); + GQuark three = g_quark_from_static_string ("three"); + GQuark keys[] = { + one, + two, + three, + }; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/2672"); + + g_datalist_init (&list); + g_datalist_id_set_data (&list, one, GINT_TO_POINTER (1)); + g_datalist_id_set_data (&list, two, GINT_TO_POINTER (2)); + g_datalist_id_set_data (&list, three, GINT_TO_POINTER (3)); + + destroy_count = 0; + g_datalist_foreach (&list, (GDataForeachFunc) notify, NULL); + g_assert_cmpint (destroy_count, ==, 3); + + g_datalist_id_remove_multiple (&list, keys, G_N_ELEMENTS (keys)); + + destroy_count = 0; + g_datalist_foreach (&list, (GDataForeachFunc) notify, NULL); + g_assert_cmpint (destroy_count, ==, 0); +} + +static void +destroy_func (gpointer data) +{ + destroy_count++; + g_assert_cmpint (GPOINTER_TO_INT (data), ==, destroy_count); +} + +static void +test_datalist_id_remove_multiple_destroy_order (void) +{ + /* Test that destroy-funcs are called in the order that the keys are + * specified, not the order that they are found in the datalist. */ + GData *list = NULL; + GQuark one = g_quark_from_static_string ("one"); + GQuark two = g_quark_from_static_string ("two"); + GQuark three = g_quark_from_static_string ("three"); + GQuark keys[] = { + one, + two, + three, + }; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/2672"); + + g_datalist_init (&list); + + g_datalist_id_set_data_full (&list, two, GINT_TO_POINTER (2), destroy_func); + g_datalist_id_set_data_full (&list, three, GINT_TO_POINTER (3), destroy_func); + g_datalist_id_set_data_full (&list, one, GINT_TO_POINTER (1), destroy_func); + + destroy_count = 0; + g_datalist_id_remove_multiple (&list, keys, G_N_ELEMENTS (keys)); + /* This verifies that destroy_func() was called three times: */ + g_assert_cmpint (destroy_count, ==, 3); +} + int main (int argc, char *argv[]) { @@ -265,6 +334,9 @@ main (int argc, char *argv[]) g_test_add_func ("/datalist/basic", test_datalist_basic); g_test_add_func ("/datalist/id", test_datalist_id); g_test_add_func ("/datalist/recursive-clear", test_datalist_clear); + g_test_add_func ("/datalist/id-remove-multiple", test_datalist_id_remove_multiple); + g_test_add_func ("/datalist/id-remove-multiple-destroy-order", + test_datalist_id_remove_multiple_destroy_order); return g_test_run (); } diff --git a/gobject/gobject.c b/gobject/gobject.c index ac887f387..5789e79a7 100644 --- a/gobject/gobject.c +++ b/gobject/gobject.c @@ -5049,16 +5049,22 @@ g_weak_ref_set (GWeakRef *weak_ref, /* Remove the weak ref from the old object */ if (old_object != NULL) { + gboolean in_weak_refs_notify; + weak_locations = g_datalist_id_get_data (&old_object->qdata, quark_weak_locations); + in_weak_refs_notify = g_datalist_id_get_data (&old_object->qdata, quark_weak_refs) == NULL; /* for it to point to an object, the object must have had it added once */ - g_assert (weak_locations != NULL); + g_assert (weak_locations != NULL || in_weak_refs_notify); - *weak_locations = g_slist_remove (*weak_locations, weak_ref); - - if (!*weak_locations) + if (weak_locations != NULL) { - weak_locations_free_unlocked (weak_locations); - g_datalist_id_remove_no_notify (&old_object->qdata, quark_weak_locations); + *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); + } } } diff --git a/gobject/tests/binding.c b/gobject/tests/binding.c index b8373e345..cc6e65987 100644 --- a/gobject/tests/binding.c +++ b/gobject/tests/binding.c @@ -1089,6 +1089,52 @@ binding_concurrent_finalizing (void) } } +static void +binding_dispose_source (void) +{ + /* Test that the source can be disposed */ + BindingSource *source = g_object_new (binding_source_get_type (), NULL); + BindingTarget *target = g_object_new (binding_target_get_type (), NULL); + GBinding *binding; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2676"); + + binding = g_object_bind_property (source, "foo", + target, "bar", + G_BINDING_DEFAULT); + + g_object_add_weak_pointer (G_OBJECT (binding), (gpointer *) &binding); + + g_object_run_dispose (G_OBJECT (source)); + g_assert_null (binding); + + g_object_unref (target); + g_object_unref (source); +} + +static void +binding_dispose_target (void) +{ + /* Test that the target can be disposed */ + BindingSource *source = g_object_new (binding_source_get_type (), NULL); + BindingTarget *target = g_object_new (binding_target_get_type (), NULL); + GBinding *binding; + + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2676"); + + binding = g_object_bind_property (source, "foo", + target, "bar", + G_BINDING_DEFAULT); + + g_object_add_weak_pointer (G_OBJECT (binding), (gpointer *) &binding); + + g_object_run_dispose (G_OBJECT (target)); + g_assert_null (binding); + + g_object_unref (target); + g_object_unref (source); +} + int main (int argc, char *argv[]) { @@ -1111,6 +1157,8 @@ main (int argc, char *argv[]) g_test_add_func ("/binding/interface", binding_interface); g_test_add_func ("/binding/concurrent-unbind", binding_concurrent_unbind); g_test_add_func ("/binding/concurrent-finalizing", binding_concurrent_finalizing); + g_test_add_func ("/binding/dispose-source", binding_dispose_source); + g_test_add_func ("/binding/dispose-target", binding_dispose_target); return g_test_run (); }