Merge branch '2672-dataset-tests-and-fixes' into 'main'

gdataset: Preserve destruction order

Closes #2672 and #2676

See merge request GNOME/glib!2776

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)
+              if (data < data_last)
-                {
+                *data = *data_last;
-                  data_end--;
-                  *data = *data_end;
-                }
 
+              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;
                 }
             }
-
+          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);
             }

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 ();
 }

gobject/gobject.c

@@ -5049,10 +5049,15 @@ g_weak_ref_set (GWeakRef *weak_ref,
       /* Remove the weak ref from the old object */
       if (old_object != NULL)
         {
-          weak_locations = g_datalist_id_get_data (&old_object->qdata, quark_weak_locations);
+          gboolean in_weak_refs_notify;
-          /* for it to point to an object, the object must have had it added once */
-          g_assert (weak_locations != NULL);
 
+          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 || in_weak_refs_notify);
+
+          if (weak_locations != NULL)
+            {
               *weak_locations = g_slist_remove (*weak_locations, weak_ref);
 
               if (!*weak_locations)
@@ -5061,6 +5066,7 @@ g_weak_ref_set (GWeakRef *weak_ref,
                   g_datalist_id_remove_no_notify (&old_object->qdata, quark_weak_locations);
                 }
             }
+        }
 
       /* Add the weak ref to the new object */
       if (new_object != NULL)

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 ();
 }