diff --git a/gobject/gobject.c b/gobject/gobject.c index e71547c19..e7f9d05f7 100644 --- a/gobject/gobject.c +++ b/gobject/gobject.c @@ -1176,26 +1176,53 @@ object_get_optional_flags (GObject *object) #endif } +/* Variant of object_get_optional_flags for when + * we know that we have exclusive access (during + * construction) + */ +static inline guint +object_get_optional_flags_X (GObject *object) +{ +#ifdef HAVE_OPTIONAL_FLAGS + GObjectReal *real = (GObjectReal *)object; + return real->optional_flags; +#else + return 0; +#endif +} + +#ifdef HAVE_OPTIONAL_FLAGS static inline void object_set_optional_flags (GObject *object, guint flags) { -#ifdef HAVE_OPTIONAL_FLAGS GObjectReal *real = (GObjectReal *)object; g_atomic_int_or (&real->optional_flags, flags); -#endif } +/* Variant for when we have exclusive access + * (during construction) + */ static inline void -object_unset_optional_flags (GObject *object, - guint flags) +object_set_optional_flags_X (GObject *object, + guint flags) { -#ifdef HAVE_OPTIONAL_FLAGS GObjectReal *real = (GObjectReal *)object; - g_atomic_int_and (&real->optional_flags, ~flags); -#endif + real->optional_flags |= flags; } +/* Variant for when we have exclusive access + * (during construction) + */ +static inline void +object_unset_optional_flags_X (GObject *object, + guint flags) +{ + GObjectReal *real = (GObjectReal *)object; + real->optional_flags &= ~flags; +} +#endif + gboolean _g_object_has_signal_handler (GObject *object) { @@ -1217,6 +1244,17 @@ _g_object_has_notify_handler (GObject *object) #endif } +static inline gboolean +_g_object_has_notify_handler_X (GObject *object) +{ +#ifdef HAVE_OPTIONAL_FLAGS + return CLASS_HAS_NOTIFY (G_OBJECT_GET_CLASS (object)) || + (object_get_optional_flags_X (object) & OPTIONAL_FLAG_HAS_NOTIFY_HANDLER) != 0; +#else + return TRUE; +#endif +} + void _g_object_set_has_signal_handler (GObject *object, guint signal_id) @@ -1243,7 +1281,7 @@ static inline void set_object_in_construction (GObject *object) { #ifdef HAVE_OPTIONAL_FLAGS - object_set_optional_flags (object, OPTIONAL_FLAG_IN_CONSTRUCTION); + object_set_optional_flags_X (object, OPTIONAL_FLAG_IN_CONSTRUCTION); #else g_datalist_id_set_data (&object->qdata, quark_in_construction, object); #endif @@ -1253,7 +1291,7 @@ static inline void unset_object_in_construction (GObject *object) { #ifdef HAVE_OPTIONAL_FLAGS - object_unset_optional_flags (object, OPTIONAL_FLAG_IN_CONSTRUCTION); + object_unset_optional_flags_X (object, OPTIONAL_FLAG_IN_CONSTRUCTION); #else g_datalist_id_set_data (&object->qdata, quark_in_construction, NULL); #endif @@ -1272,11 +1310,8 @@ g_object_init (GObject *object, g_object_notify_queue_freeze (object, FALSE); } - if (CLASS_HAS_CUSTOM_CONSTRUCTOR (class)) - { - /* mark object in-construction for notify_queue_thaw() and to allow construct-only properties */ - set_object_in_construction (object); - } + /* mark object in-construction for notify_queue_thaw() and to allow construct-only properties */ + set_object_in_construction (object); GOBJECT_IF_DEBUG (OBJECTS, { @@ -1444,24 +1479,51 @@ static inline void g_object_notify_by_spec_internal (GObject *object, GParamSpec *pspec) { +#ifdef HAVE_OPTIONAL_FLAGS + guint object_flags; +#endif + gboolean has_notify; + gboolean in_init; + if (G_UNLIKELY (~pspec->flags & G_PARAM_READABLE)) return; param_spec_follow_override (&pspec); - if (pspec != NULL && - _g_object_has_notify_handler (object)) +#ifdef HAVE_OPTIONAL_FLAGS + /* get all flags we need with a single atomic read */ + object_flags = object_get_optional_flags (object); + has_notify = ((object_flags & OPTIONAL_FLAG_HAS_NOTIFY_HANDLER) != 0) || + CLASS_HAS_NOTIFY (G_OBJECT_GET_CLASS (object)); + in_init = (object_flags & OPTIONAL_FLAG_IN_CONSTRUCTION) != 0; +#else + has_notify = TRUE; + in_init = object_in_construction (object); +#endif + + if (pspec != NULL && has_notify) { GObjectNotifyQueue *nqueue; + gboolean need_thaw = TRUE; /* conditional freeze: only increase freeze count if already frozen */ nqueue = g_object_notify_queue_freeze (object, TRUE); + if (in_init && !nqueue) + { + /* We did not freeze the queue in g_object_init, but + * we gained a notify handler in instance init, so + * now we need to freeze just-in-time + */ + nqueue = g_object_notify_queue_freeze (object, FALSE); + need_thaw = FALSE; + } if (nqueue != NULL) { /* we're frozen, so add to the queue and release our freeze */ g_object_notify_queue_add (object, nqueue, pspec); - g_object_notify_queue_thaw (object, nqueue); + if (need_thaw) + g_object_notify_queue_thaw (object, nqueue); } else { @@ -2108,28 +2170,16 @@ g_object_new_with_custom_constructor (GObjectClass *class, if (CLASS_HAS_PROPS (class)) { - /* If this object was newly_constructed then g_object_init() - * froze the queue. We need to freeze it here in order to get - * the handle so that we can thaw it below (otherwise it will - * be frozen forever). - * - * We also want to do a freeze if we have any params to set, - * even on a non-newly_constructed object. - * - * It's possible that we have the case of non-newly created - * singleton and all of the passed-in params were construct - * properties so n_params > 0 but we will actually set no - * properties. This is a pretty lame case to optimise, so - * just ignore it and freeze anyway. - */ - if (newly_constructed || n_params) - nqueue = g_object_notify_queue_freeze (object, FALSE); - - /* Remember: if it was newly_constructed then g_object_init() - * already did a freeze, so we now have two. Release one. - */ - if (newly_constructed && CLASS_HAS_NOTIFY (class)) - g_object_notify_queue_thaw (object, nqueue); + if ((newly_constructed && _g_object_has_notify_handler_X (object)) || + _g_object_has_notify_handler (object)) + { + /* This may or may not have been setup in g_object_init(). + * If it hasn't, we do it now. + */ + nqueue = g_datalist_id_get_data (&object->qdata, quark_notify_queue); + if (!nqueue) + nqueue = g_object_notify_queue_freeze (object, FALSE); + } } /* run 'constructed' handler if there is a custom one */ @@ -2164,15 +2214,20 @@ g_object_new_internal (GObjectClass *class, g_assert (g_object_is_aligned (object)); + unset_object_in_construction (object); + if (CLASS_HAS_PROPS (class)) { GSList *node; - if (CLASS_HAS_NOTIFY (class)) + if (_g_object_has_notify_handler_X (object)) { - /* This will have been setup in g_object_init() */ + /* This may or may not have been setup in g_object_init(). + * If it hasn't, we do it now. + */ nqueue = g_datalist_id_get_data (&object->qdata, quark_notify_queue); - g_assert (nqueue != NULL); + if (!nqueue) + nqueue = g_object_notify_queue_freeze (object, FALSE); } /* We will set exactly n_construct_properties construct diff --git a/gobject/tests/meson.build b/gobject/tests/meson.build index 84132518a..f701152ca 100644 --- a/gobject/tests/meson.build +++ b/gobject/tests/meson.build @@ -29,6 +29,8 @@ marshalers_c = custom_target('marshalers_c', ) gobject_tests = { + 'notify-init' : {}, + 'notify-init2' : {}, 'qdata' : {}, 'accumulator' : { 'source' : ['accumulator.c', marshalers_h, marshalers_c], diff --git a/gobject/tests/notify-init.c b/gobject/tests/notify-init.c new file mode 100644 index 000000000..99c628593 --- /dev/null +++ b/gobject/tests/notify-init.c @@ -0,0 +1,268 @@ +/* GLib testing framework examples and tests + * Copyright (C) 2022 Red Hat, Inc. + * + * This work is provided "as is"; redistribution and modification + * in whole or in part, in any medium, physical or electronic is + * permitted without restriction. + * + * This work is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * + * In no event shall the authors or contributors be liable for any + * direct, indirect, incidental, special, exemplary, or consequential + * damages (including, but not limited to, procurement of substitute + * goods or services; loss of use, data, or profits; or business + * interruption) however caused and on any theory of liability, whether + * in contract, strict liability, or tort (including negligence or + * otherwise) arising in any way out of the use of this software, even + * if advised of the possibility of such damage. + */ + +#include +#include +#include + +typedef struct { + GObject parent_instance; + gint foo; + gboolean bar; + gchar *baz; + gchar *quux; +} TestObject; + +typedef struct { + GObjectClass parent_class; +} TestObjectClass; + +typedef enum { + PROP_FOO = 1, + PROP_BAR, + PROP_BAZ, + PROP_QUUX, + N_PROPERTIES +} TestObjectProperty; + +static GParamSpec *properties[N_PROPERTIES] = { NULL, }; + +static GType test_object_get_type (void); +G_DEFINE_TYPE (TestObject, test_object, G_TYPE_OBJECT) + +static void +test_object_set_foo (TestObject *obj, + gint foo) +{ + if (obj->foo != foo) + { + obj->foo = foo; + + g_assert (properties[PROP_FOO] != NULL); + g_object_notify_by_pspec (G_OBJECT (obj), properties[PROP_FOO]); + } +} + +static void +test_object_set_bar (TestObject *obj, + gboolean bar) +{ + bar = !!bar; + + if (obj->bar != bar) + { + obj->bar = bar; + + g_assert (properties[PROP_BAR] != NULL); + g_object_notify_by_pspec (G_OBJECT (obj), properties[PROP_BAR]); + } +} + +static void +test_object_set_baz (TestObject *obj, + const gchar *baz) +{ + if (g_strcmp0 (obj->baz, baz) != 0) + { + g_free (obj->baz); + obj->baz = g_strdup (baz); + + g_assert (properties[PROP_BAZ] != NULL); + g_object_notify_by_pspec (G_OBJECT (obj), properties[PROP_BAZ]); + } +} + +static void +test_object_set_quux (TestObject *obj, + const gchar *quux) +{ + if (g_strcmp0 (obj->quux, quux) != 0) + { + g_free (obj->quux); + obj->quux = g_strdup (quux); + + g_assert (properties[PROP_QUUX] != NULL); + g_object_notify_by_pspec (G_OBJECT (obj), properties[PROP_QUUX]); + } +} + +static void +test_object_finalize (GObject *gobject) +{ + TestObject *self = (TestObject *) gobject; + + g_free (self->baz); + g_free (self->quux); + + G_OBJECT_CLASS (test_object_parent_class)->finalize (gobject); +} + +static void +test_object_set_property (GObject *gobject, + guint prop_id, + const GValue *value, + GParamSpec *pspec) +{ + TestObject *tobj = (TestObject *) gobject; + + g_assert_cmpint (prop_id, !=, 0); + g_assert_cmpint (prop_id, !=, N_PROPERTIES); + g_assert (pspec == properties[prop_id]); + + switch ((TestObjectProperty)prop_id) + { + case PROP_FOO: + test_object_set_foo (tobj, g_value_get_int (value)); + break; + + case PROP_BAR: + test_object_set_bar (tobj, g_value_get_boolean (value)); + break; + + case PROP_BAZ: + test_object_set_baz (tobj, g_value_get_string (value)); + break; + + case PROP_QUUX: + test_object_set_quux (tobj, g_value_get_string (value)); + break; + + default: + g_assert_not_reached (); + } +} + +static void +test_object_get_property (GObject *gobject, + guint prop_id, + GValue *value, + GParamSpec *pspec) +{ + TestObject *tobj = (TestObject *) gobject; + + g_assert_cmpint (prop_id, !=, 0); + g_assert_cmpint (prop_id, !=, N_PROPERTIES); + g_assert (pspec == properties[prop_id]); + + switch ((TestObjectProperty)prop_id) + { + case PROP_FOO: + g_value_set_int (value, tobj->foo); + break; + + case PROP_BAR: + g_value_set_boolean (value, tobj->bar); + break; + + case PROP_BAZ: + g_value_set_string (value, tobj->baz); + break; + + case PROP_QUUX: + g_value_set_string (value, tobj->quux); + break; + + default: + g_assert_not_reached (); + } +} + +static void +test_object_class_init (TestObjectClass *klass) +{ + GObjectClass *gobject_class = G_OBJECT_CLASS (klass); + + properties[PROP_FOO] = g_param_spec_int ("foo", "Foo", "Foo", + -1, G_MAXINT, + 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + properties[PROP_BAR] = g_param_spec_boolean ("bar", "Bar", "Bar", + FALSE, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + properties[PROP_BAZ] = g_param_spec_string ("baz", "Baz", "Baz", + NULL, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + + properties[PROP_QUUX] = g_param_spec_string ("quux", "quux", "quux", + NULL, + G_PARAM_READWRITE | G_PARAM_EXPLICIT_NOTIFY | G_PARAM_STATIC_STRINGS); + + gobject_class->set_property = test_object_set_property; + gobject_class->get_property = test_object_get_property; + gobject_class->finalize = test_object_finalize; + + g_object_class_install_properties (gobject_class, N_PROPERTIES, properties); +} + +static void +quux_changed (TestObject *self, + GParamSpec *pspec, + gpointer data) +{ + g_assert (self->baz != NULL); +} + +static void +test_object_init (TestObject *self) +{ + /* This instance init behavior is the thing we are testing: + * + * 1. Connect to notify::quux + * 2. Change the the quux property + * 3. Continue to set up things that the quux_changed handler + * relies on + * + * The expected behavior is that: + * + * - The quux_changed handler *is* called + * - It is only called after the object is fully constructed + */ + g_signal_connect (self, "notify::quux", G_CALLBACK (quux_changed), NULL); + + test_object_set_quux (self, "quux"); + + self->foo = 42; + self->bar = TRUE; + self->baz = g_strdup ("Hello"); +} + +static void +test_notify_in_init (void) +{ + TestObject *obj; + + g_test_summary ("Test that emitting notify with a handler already connected in test_object_init() works"); + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2665"); + + obj = g_object_new (test_object_get_type (), NULL); + + g_object_unref (obj); +} + +int +main (int argc, char *argv[]) +{ + g_test_init (&argc, &argv, NULL); + + g_test_add_func ("/properties/notify-in-init", test_notify_in_init); + + return g_test_run (); +} diff --git a/gobject/tests/notify-init2.c b/gobject/tests/notify-init2.c new file mode 100644 index 000000000..ab6677c1d --- /dev/null +++ b/gobject/tests/notify-init2.c @@ -0,0 +1,253 @@ +/* GLib testing framework examples and tests + * Copyright (C) 2022 Red Hat, Inc. + * + * This work is provided "as is"; redistribution and modification + * in whole or in part, in any medium, physical or electronic is + * permitted without restriction. + * + * This work is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * + * In no event shall the authors or contributors be liable for any + * direct, indirect, incidental, special, exemplary, or consequential + * damages (including, but not limited to, procurement of substitute + * goods or services; loss of use, data, or profits; or business + * interruption) however caused and on any theory of liability, whether + * in contract, strict liability, or tort (including negligence or + * otherwise) arising in any way out of the use of this software, even + * if advised of the possibility of such damage. + */ + +#include +#include +#include + +typedef struct { + GObject parent_instance; + gint foo; + gboolean bar; + gchar *baz; + gchar *quux; +} TestObject; + +typedef struct { + GObjectClass parent_class; +} TestObjectClass; + +typedef enum { + PROP_FOO = 1, + PROP_BAR, + PROP_BAZ, + PROP_QUUX, + N_PROPERTIES +} TestObjectProperty; + +static GParamSpec *properties[N_PROPERTIES] = { NULL, }; + +static GType test_object_get_type (void); +G_DEFINE_TYPE (TestObject, test_object, G_TYPE_OBJECT) + +static void +test_object_set_foo (TestObject *obj, + gint foo) +{ + if (obj->foo != foo) + { + obj->foo = foo; + + g_assert (properties[PROP_FOO] != NULL); + g_object_notify_by_pspec (G_OBJECT (obj), properties[PROP_FOO]); + } +} + +static void +test_object_set_bar (TestObject *obj, + gboolean bar) +{ + bar = !!bar; + + if (obj->bar != bar) + { + obj->bar = bar; + + g_assert (properties[PROP_BAR] != NULL); + g_object_notify_by_pspec (G_OBJECT (obj), properties[PROP_BAR]); + } +} + +static void +test_object_set_baz (TestObject *obj, + const gchar *baz) +{ + if (g_strcmp0 (obj->baz, baz) != 0) + { + g_free (obj->baz); + obj->baz = g_strdup (baz); + + g_assert (properties[PROP_BAZ] != NULL); + g_object_notify_by_pspec (G_OBJECT (obj), properties[PROP_BAZ]); + } +} + +static void +test_object_set_quux (TestObject *obj, + const gchar *quux) +{ + if (g_strcmp0 (obj->quux, quux) != 0) + { + g_free (obj->quux); + obj->quux = g_strdup (quux); + + g_assert (properties[PROP_QUUX] != NULL); + g_object_notify_by_pspec (G_OBJECT (obj), properties[PROP_QUUX]); + } +} + +static void +test_object_finalize (GObject *gobject) +{ + TestObject *self = (TestObject *) gobject; + + g_free (self->baz); + g_free (self->quux); + + G_OBJECT_CLASS (test_object_parent_class)->finalize (gobject); +} + +static GObject * +test_object_constructor (GType type, + guint n_construct_properties, + GObjectConstructParam *construct_properties) +{ + return G_OBJECT_CLASS (test_object_parent_class)->constructor (type, n_construct_properties, construct_properties); +} + +static void +test_object_set_property (GObject *gobject, + guint prop_id, + const GValue *value, + GParamSpec *pspec) +{ + TestObject *tobj = (TestObject *) gobject; + + g_assert_cmpint (prop_id, !=, 0); + g_assert_cmpint (prop_id, !=, N_PROPERTIES); + g_assert (pspec == properties[prop_id]); + + switch ((TestObjectProperty)prop_id) + { + case PROP_FOO: + test_object_set_foo (tobj, g_value_get_int (value)); + break; + + case PROP_BAR: + test_object_set_bar (tobj, g_value_get_boolean (value)); + break; + + case PROP_BAZ: + test_object_set_baz (tobj, g_value_get_string (value)); + break; + + case PROP_QUUX: + test_object_set_quux (tobj, g_value_get_string (value)); + break; + + default: + g_assert_not_reached (); + } +} + +static void +test_object_get_property (GObject *gobject, + guint prop_id, + GValue *value, + GParamSpec *pspec) +{ + TestObject *tobj = (TestObject *) gobject; + + g_assert_cmpint (prop_id, !=, 0); + g_assert_cmpint (prop_id, !=, N_PROPERTIES); + g_assert (pspec == properties[prop_id]); + + switch ((TestObjectProperty)prop_id) + { + case PROP_FOO: + g_value_set_int (value, tobj->foo); + break; + + case PROP_BAR: + g_value_set_boolean (value, tobj->bar); + break; + + case PROP_BAZ: + g_value_set_string (value, tobj->baz); + break; + + case PROP_QUUX: + g_value_set_string (value, tobj->quux); + break; + + default: + g_assert_not_reached (); + } +} + +static void +test_object_class_init (TestObjectClass *klass) +{ + GObjectClass *gobject_class = G_OBJECT_CLASS (klass); + + properties[PROP_FOO] = g_param_spec_int ("foo", "Foo", "Foo", + -1, G_MAXINT, + 0, + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | G_PARAM_STATIC_STRINGS); + properties[PROP_BAR] = g_param_spec_boolean ("bar", "Bar", "Bar", + FALSE, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + properties[PROP_BAZ] = g_param_spec_string ("baz", "Baz", "Baz", + NULL, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + + properties[PROP_QUUX] = g_param_spec_string ("quux", "quux", "quux", + NULL, + G_PARAM_READWRITE | G_PARAM_EXPLICIT_NOTIFY | G_PARAM_STATIC_STRINGS); + + gobject_class->constructor = test_object_constructor; + gobject_class->set_property = test_object_set_property; + gobject_class->get_property = test_object_get_property; + gobject_class->finalize = test_object_finalize; + + g_object_class_install_properties (gobject_class, N_PROPERTIES, properties); +} + +static void +test_object_init (TestObject *self) +{ + self->foo = 42; + self->bar = TRUE; + self->baz = g_strdup ("Hello"); +} + +static void +test_notify_in_init (void) +{ + TestObject *obj; + + g_test_summary ("Test that notify freezing during construction of objects with custom constructor works"); + g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2665"); + + obj = g_object_new (test_object_get_type (), "bar", FALSE, NULL); + + g_object_unref (obj); +} + +int +main (int argc, char *argv[]) +{ + g_test_init (&argc, &argv, NULL); + + g_test_add_func ("/properties/notify-in-init2", test_notify_in_init); + + return g_test_run (); +}