From 3fc7c979796a18d4900ab66b33c15d1b42898e92 Mon Sep 17 00:00:00 2001 From: Jason Francis Date: Fri, 1 Apr 2022 18:27:43 -0400 Subject: [PATCH] gtk: Set widget template children to NULL before destroy unref --- gtk/gtkwidget.c | 72 +++++++++++++++++---------- testsuite/gtk/builder.c | 108 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 25 deletions(-) diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c index 3d1a46897d..5200704540 100644 --- a/gtk/gtkwidget.c +++ b/gtk/gtkwidget.c @@ -7405,6 +7405,7 @@ gtk_widget_real_destroy (GtkWidget *object) { GtkWidgetClass *class; GSList *l; + GHashTable *auto_children; #ifdef G_ENABLE_CONSISTENCY_CHECKS GSList *assertions = NULL; @@ -7462,8 +7463,52 @@ gtk_widget_real_destroy (GtkWidget *object) } #endif /* G_ENABLE_CONSISTENCY_CHECKS */ - /* Release references to all automated children */ - g_object_set_qdata (G_OBJECT (widget), quark_auto_children, NULL); + /* Prepare to release references to all automated children */ + auto_children = g_object_steal_qdata (G_OBJECT (widget), quark_auto_children); + + /* Set any automatic private data pointers to NULL and release child references */ + for (class = GTK_WIDGET_GET_CLASS (widget); + GTK_IS_WIDGET_CLASS (class); + class = g_type_class_peek_parent (class)) + { + GHashTable *auto_child_hash = NULL; + + if (!class->priv->template) + continue; + + if (auto_children) + { + GType type = G_TYPE_FROM_CLASS (class); + + g_hash_table_steal_extended (auto_children, + GSIZE_TO_POINTER (type), + NULL, + (gpointer *) &auto_child_hash); + } + + for (l = class->priv->template->children; l; l = l->next) + { + AutomaticChildClass *child_class = l->data; + + if (child_class->offset != 0) + { + gpointer field_p; + + /* Nullify instance private data for internal children */ + field_p = G_STRUCT_MEMBER_P (widget, child_class->offset); + (* (gpointer *) field_p) = NULL; + } + + /* Release the references in order after setting the pointer to NULL */ + if (auto_child_hash) + g_hash_table_remove (auto_child_hash, child_class->name); + } + + g_clear_pointer (&auto_child_hash, g_hash_table_unref); + } + + /* Free the child reference hash table */ + g_clear_pointer (&auto_children, g_hash_table_unref); #ifdef G_ENABLE_CONSISTENCY_CHECKS for (l = assertions; l; l = l->next) @@ -7481,29 +7526,6 @@ gtk_widget_real_destroy (GtkWidget *object) } g_slist_free (assertions); #endif /* G_ENABLE_CONSISTENCY_CHECKS */ - - /* Set any automatic private data pointers to NULL */ - for (class = GTK_WIDGET_GET_CLASS (widget); - GTK_IS_WIDGET_CLASS (class); - class = g_type_class_peek_parent (class)) - { - if (!class->priv->template) - continue; - - for (l = class->priv->template->children; l; l = l->next) - { - AutomaticChildClass *child_class = l->data; - - if (child_class->offset != 0) - { - gpointer field_p; - - /* Nullify instance private data for internal children */ - field_p = G_STRUCT_MEMBER_P (widget, child_class->offset); - (* (gpointer *) field_p) = NULL; - } - } - } } /* Callers of add_mnemonic_label() should disconnect on ::destroy */ diff --git a/testsuite/gtk/builder.c b/testsuite/gtk/builder.c index 88dc7c853a..bbcf08d726 100644 --- a/testsuite/gtk/builder.c +++ b/testsuite/gtk/builder.c @@ -2687,6 +2687,113 @@ test_expressions (void) } } +#define MY_GTK_BOX_TEMPLATE "\ +\n\ + \n\ +\n" + +#define MY_TYPE_GTK_BOX (my_gtk_box_get_type ()) +G_DECLARE_FINAL_TYPE (MyGtkBox, my_gtk_box, MY, GTK_BOX, GtkWidget) + +struct _MyGtkBox +{ + GtkWidget parent_instance; + GtkLabel *label; + GtkLabel *label2; +}; + +G_DEFINE_TYPE (MyGtkBox, my_gtk_box, GTK_TYPE_WIDGET); + +static void +my_gtk_box_init (MyGtkBox *grid) +{ + gtk_widget_init_template (GTK_WIDGET (grid)); +} + +static void +my_gtk_box_dispose (GObject *obj) +{ + MyGtkBox *my_gtk_box = MY_GTK_BOX (obj); + GtkWidget *child; + + while ((child = gtk_widget_get_first_child (GTK_WIDGET (my_gtk_box)))) + gtk_widget_unparent (child); + + G_OBJECT_CLASS (my_gtk_box_parent_class)->dispose (obj); +} + +static void +my_gtk_box_class_init (MyGtkBoxClass *klass) +{ + GBytes *template = g_bytes_new_static (MY_GTK_BOX_TEMPLATE, strlen (MY_GTK_BOX_TEMPLATE)); + GtkWidgetClass *widget_class = GTK_WIDGET_CLASS (klass); + + gtk_widget_class_set_template (widget_class, template); + gtk_widget_class_bind_template_child (widget_class, MyGtkBox, label); + gtk_widget_class_bind_template_child (widget_class, MyGtkBox, label2); + + G_OBJECT_CLASS (klass)->dispose = my_gtk_box_dispose; +} + +typedef struct +{ + MyGtkBox *my_gtk_box; + guint destroy_count; +} BoxDestroyData; + +static void +my_label_destroy (GtkLabel *label, BoxDestroyData *data) +{ + g_assert_true (MY_IS_GTK_BOX (data->my_gtk_box)); + /* Make sure the other label is null if it was disposed first */ + g_assert_true (!data->my_gtk_box->label2 || GTK_IS_LABEL (data->my_gtk_box->label2)); + data->destroy_count++; +} + +static void +my_label2_destroy (GtkLabel *label2, BoxDestroyData *data) +{ + g_assert_true (MY_IS_GTK_BOX (data->my_gtk_box)); + /* Make sure the other label is null if it was disposed first */ + g_assert_true (!data->my_gtk_box->label || GTK_IS_LABEL (data->my_gtk_box->label)); + data->destroy_count++; +} + +static void +test_child_dispose_order (void) +{ + BoxDestroyData data = { 0, }; + + /* make sure the type we are trying to register does not exist */ + g_assert_false (g_type_from_name ("MyGtkBox")); + + /* create the template object */ + data.my_gtk_box = g_object_ref_sink (g_object_new (MY_TYPE_GTK_BOX, NULL)); + + /* Check everything is fine */ + g_assert_true (g_type_from_name ("MyGtkBox")); + g_assert_true (MY_IS_GTK_BOX (data.my_gtk_box)); + g_assert_true (GTK_IS_LABEL (data.my_gtk_box->label)); + g_assert_true (GTK_IS_LABEL (data.my_gtk_box->label2)); + + /* Check if both labels are destroyed */ + g_signal_connect (data.my_gtk_box->label, "destroy", G_CALLBACK (my_label_destroy), &data); + g_signal_connect (data.my_gtk_box->label2, "destroy", G_CALLBACK (my_label2_destroy), &data); + g_object_unref (data.my_gtk_box); + g_assert_cmpuint (data.destroy_count, ==, 2); +} + int main (int argc, char **argv) { @@ -2734,6 +2841,7 @@ main (int argc, char **argv) g_test_add_func ("/Builder/Shortcuts", test_shortcuts); g_test_add_func ("/Builder/Transforms", test_transforms); g_test_add_func ("/Builder/Expressions", test_expressions); + g_test_add_func ("/Builder/Child Dispose Order", test_child_dispose_order); return g_test_run(); }