gobject: Be consistent in using atomic logic to handle the GParamSpecPool

We init it atomically but then we don't really use it in such way and it
may lead to races at read/write times
This commit is contained in:
Marco Trevisan (Treviño) 2025-01-31 12:30:07 +01:00
parent 589c8295ea
commit ad2041a292

View File

@ -228,7 +228,7 @@ static GQuark quark_closure_array = 0;
static GQuark quark_weak_notifies = 0; static GQuark quark_weak_notifies = 0;
static GQuark quark_toggle_refs = 0; static GQuark quark_toggle_refs = 0;
static GQuark quark_notify_queue; static GQuark quark_notify_queue;
static GParamSpecPool *pspec_pool = NULL; static GParamSpecPool *pspec_pool = NULL; /* atomic */
static gulong gobject_signals[LAST_SIGNAL] = { 0, }; static gulong gobject_signals[LAST_SIGNAL] = { 0, };
static guint (*floating_flag_handler) (GObject*, gint) = object_floating_flag_handler; static guint (*floating_flag_handler) (GObject*, gint) = object_floating_flag_handler;
static GQuark quark_weak_locations = 0; static GQuark quark_weak_locations = 0;
@ -919,15 +919,22 @@ _g_object_type_init (void)
* first caller of this function will win the race. Any other access to * first caller of this function will win the race. Any other access to
* the GParamSpecPool is done under its own mutex. * the GParamSpecPool is done under its own mutex.
*/ */
static inline void static inline GParamSpecPool *
g_object_init_pspec_pool (void) g_object_maybe_init_pspec_pool (void)
{ {
if (G_UNLIKELY (g_atomic_pointer_get (&pspec_pool) == NULL)) GParamSpecPool *pool = g_atomic_pointer_get (&pspec_pool);
if (G_UNLIKELY (pool == NULL))
{ {
GParamSpecPool *pool = g_param_spec_pool_new (TRUE); GParamSpecPool *new_pool = g_param_spec_pool_new (TRUE);
if (!g_atomic_pointer_compare_and_exchange (&pspec_pool, NULL, pool)) if (g_atomic_pointer_compare_and_exchange_full (&pspec_pool, NULL,
g_param_spec_pool_free (pool); new_pool, &pool))
pool = g_steal_pointer (&new_pool);
g_clear_pointer (&new_pool, g_param_spec_pool_free);
} }
return pool;
} }
static void static void
@ -954,18 +961,19 @@ static void
g_object_base_class_finalize (GObjectClass *class) g_object_base_class_finalize (GObjectClass *class)
{ {
GList *list, *node; GList *list, *node;
GParamSpecPool *param_spec_pool;
_g_signals_destroy (G_OBJECT_CLASS_TYPE (class)); _g_signals_destroy (G_OBJECT_CLASS_TYPE (class));
g_slist_free (class->construct_properties); g_slist_free (class->construct_properties);
class->construct_properties = NULL; class->construct_properties = NULL;
class->n_construct_properties = 0; class->n_construct_properties = 0;
list = g_param_spec_pool_list_owned (pspec_pool, G_OBJECT_CLASS_TYPE (class)); param_spec_pool = g_atomic_pointer_get (&pspec_pool);
list = g_param_spec_pool_list_owned (param_spec_pool, G_OBJECT_CLASS_TYPE (class));
for (node = list; node; node = node->next) for (node = list; node; node = node->next)
{ {
GParamSpec *pspec = node->data; GParamSpec *pspec = node->data;
g_param_spec_pool_remove (param_spec_pool, pspec);
g_param_spec_pool_remove (pspec_pool, pspec);
PARAM_SPEC_SET_PARAM_ID (pspec, 0); PARAM_SPEC_SET_PARAM_ID (pspec, 0);
g_param_spec_unref (pspec); g_param_spec_unref (pspec);
} }
@ -981,7 +989,7 @@ g_object_do_class_init (GObjectClass *class)
quark_toggle_refs = g_quark_from_static_string ("GObject-toggle-references"); quark_toggle_refs = g_quark_from_static_string ("GObject-toggle-references");
quark_notify_queue = g_quark_from_static_string ("GObject-notify-queue"); quark_notify_queue = g_quark_from_static_string ("GObject-notify-queue");
g_object_init_pspec_pool (); g_object_maybe_init_pspec_pool ();
class->constructor = g_object_constructor; class->constructor = g_object_constructor;
class->constructed = g_object_constructed; class->constructed = g_object_constructed;
@ -1049,11 +1057,12 @@ install_property_internal (GType g_type,
guint property_id, guint property_id,
GParamSpec *pspec) GParamSpec *pspec)
{ {
GParamSpecPool *param_spec_pool;
g_param_spec_ref_sink (pspec); g_param_spec_ref_sink (pspec);
g_object_init_pspec_pool (); param_spec_pool = g_object_maybe_init_pspec_pool ();
if (g_param_spec_pool_lookup (pspec_pool, pspec->name, g_type, FALSE)) if (g_param_spec_pool_lookup (param_spec_pool, pspec->name, g_type, FALSE))
{ {
g_critical ("When installing property: type '%s' already has a property named '%s'", g_critical ("When installing property: type '%s' already has a property named '%s'",
g_type_name (g_type), g_type_name (g_type),
@ -1063,7 +1072,7 @@ install_property_internal (GType g_type,
} }
PARAM_SPEC_SET_PARAM_ID (pspec, property_id); PARAM_SPEC_SET_PARAM_ID (pspec, property_id);
g_param_spec_pool_insert (pspec_pool, g_steal_pointer (&pspec), g_type); g_param_spec_pool_insert (param_spec_pool, g_steal_pointer (&pspec), g_type);
return TRUE; return TRUE;
} }
@ -1116,7 +1125,8 @@ validate_and_install_class_property (GObjectClass *class,
/* for property overrides of construct properties, we have to get rid /* for property overrides of construct properties, we have to get rid
* of the overridden inherited construct property * of the overridden inherited construct property
*/ */
pspec = g_param_spec_pool_lookup (pspec_pool, pspec->name, parent_type, TRUE); pspec = g_param_spec_pool_lookup (g_atomic_pointer_get (&pspec_pool),
pspec->name, parent_type, TRUE);
if (pspec && pspec->flags & (G_PARAM_CONSTRUCT | G_PARAM_CONSTRUCT_ONLY)) if (pspec && pspec->flags & (G_PARAM_CONSTRUCT | G_PARAM_CONSTRUCT_ONLY))
{ {
class->construct_properties = g_slist_remove (class->construct_properties, pspec); class->construct_properties = g_slist_remove (class->construct_properties, pspec);
@ -1232,7 +1242,7 @@ find_pspec (GObjectClass *class,
} }
} }
return g_param_spec_pool_lookup (pspec_pool, return g_param_spec_pool_lookup (g_atomic_pointer_get (&pspec_pool),
property_name, property_name,
((GTypeClass *)class)->g_type, ((GTypeClass *)class)->g_type,
TRUE); TRUE);
@ -1475,13 +1485,14 @@ g_object_interface_find_property (gpointer g_iface,
const gchar *property_name) const gchar *property_name)
{ {
GTypeInterface *iface_class = g_iface; GTypeInterface *iface_class = g_iface;
GParamSpecPool *param_spec_pool;
g_return_val_if_fail (G_TYPE_IS_INTERFACE (iface_class->g_type), NULL); g_return_val_if_fail (G_TYPE_IS_INTERFACE (iface_class->g_type), NULL);
g_return_val_if_fail (property_name != NULL, NULL); g_return_val_if_fail (property_name != NULL, NULL);
g_object_init_pspec_pool (); param_spec_pool = g_object_maybe_init_pspec_pool ();
return g_param_spec_pool_lookup (pspec_pool, return g_param_spec_pool_lookup (param_spec_pool,
property_name, property_name,
iface_class->g_type, iface_class->g_type,
FALSE); FALSE);
@ -1518,6 +1529,7 @@ g_object_class_override_property (GObjectClass *oclass,
guint property_id, guint property_id,
const gchar *name) const gchar *name)
{ {
GParamSpecPool *param_spec_pool;
GParamSpec *overridden = NULL; GParamSpec *overridden = NULL;
GParamSpec *new; GParamSpec *new;
GType parent_type; GType parent_type;
@ -1526,11 +1538,13 @@ g_object_class_override_property (GObjectClass *oclass,
g_return_if_fail (property_id > 0); g_return_if_fail (property_id > 0);
g_return_if_fail (name != NULL); g_return_if_fail (name != NULL);
param_spec_pool = g_atomic_pointer_get (&pspec_pool);
/* Find the overridden property; first check parent types /* Find the overridden property; first check parent types
*/ */
parent_type = g_type_parent (G_OBJECT_CLASS_TYPE (oclass)); parent_type = g_type_parent (G_OBJECT_CLASS_TYPE (oclass));
if (parent_type != G_TYPE_NONE) if (parent_type != G_TYPE_NONE)
overridden = g_param_spec_pool_lookup (pspec_pool, overridden = g_param_spec_pool_lookup (param_spec_pool,
name, name,
parent_type, parent_type,
TRUE); TRUE);
@ -1544,7 +1558,7 @@ g_object_class_override_property (GObjectClass *oclass,
ifaces = g_type_interfaces (G_OBJECT_CLASS_TYPE (oclass), &n_ifaces); ifaces = g_type_interfaces (G_OBJECT_CLASS_TYPE (oclass), &n_ifaces);
while (n_ifaces-- && !overridden) while (n_ifaces-- && !overridden)
{ {
overridden = g_param_spec_pool_lookup (pspec_pool, overridden = g_param_spec_pool_lookup (param_spec_pool,
name, name,
ifaces[n_ifaces], ifaces[n_ifaces],
FALSE); FALSE);
@ -1583,7 +1597,7 @@ g_object_class_list_properties (GObjectClass *class,
g_return_val_if_fail (G_IS_OBJECT_CLASS (class), NULL); g_return_val_if_fail (G_IS_OBJECT_CLASS (class), NULL);
pspecs = g_param_spec_pool_list (pspec_pool, pspecs = g_param_spec_pool_list (g_atomic_pointer_get (&pspec_pool),
G_OBJECT_CLASS_TYPE (class), G_OBJECT_CLASS_TYPE (class),
&n); &n);
if (n_properties_p) if (n_properties_p)
@ -1616,14 +1630,15 @@ g_object_interface_list_properties (gpointer g_iface,
guint *n_properties_p) guint *n_properties_p)
{ {
GTypeInterface *iface_class = g_iface; GTypeInterface *iface_class = g_iface;
GParamSpecPool *param_spec_pool;
GParamSpec **pspecs; GParamSpec **pspecs;
guint n; guint n;
g_return_val_if_fail (G_TYPE_IS_INTERFACE (iface_class->g_type), NULL); g_return_val_if_fail (G_TYPE_IS_INTERFACE (iface_class->g_type), NULL);
g_object_init_pspec_pool (); param_spec_pool = g_object_maybe_init_pspec_pool ();
pspecs = g_param_spec_pool_list (pspec_pool, pspecs = g_param_spec_pool_list (param_spec_pool,
iface_class->g_type, iface_class->g_type,
&n); &n);
if (n_properties_p) if (n_properties_p)
@ -1960,7 +1975,7 @@ g_object_notify (GObject *object,
* (by, e.g. calling g_object_class_find_property()) * (by, e.g. calling g_object_class_find_property())
* because g_object_notify_queue_add() does that * because g_object_notify_queue_add() does that
*/ */
pspec = g_param_spec_pool_lookup (pspec_pool, pspec = g_param_spec_pool_lookup (g_atomic_pointer_get (&pspec_pool),
property_name, property_name,
G_OBJECT_TYPE (object), G_OBJECT_TYPE (object),
TRUE); TRUE);
@ -2213,6 +2228,7 @@ object_interface_check_properties (gpointer check_data,
{ {
GTypeInterface *iface_class = g_iface; GTypeInterface *iface_class = g_iface;
GObjectClass *class; GObjectClass *class;
GParamSpecPool *param_spec_pool;
GType iface_type = iface_class->g_type; GType iface_type = iface_class->g_type;
GParamSpec **pspecs; GParamSpec **pspecs;
guint n; guint n;
@ -2225,11 +2241,12 @@ object_interface_check_properties (gpointer check_data,
if (!G_IS_OBJECT_CLASS (class)) if (!G_IS_OBJECT_CLASS (class))
goto out; goto out;
pspecs = g_param_spec_pool_list (pspec_pool, iface_type, &n); param_spec_pool = g_atomic_pointer_get (&pspec_pool);
pspecs = g_param_spec_pool_list (param_spec_pool, iface_type, &n);
while (n--) while (n--)
{ {
GParamSpec *class_pspec = g_param_spec_pool_lookup (pspec_pool, GParamSpec *class_pspec = g_param_spec_pool_lookup (param_spec_pool,
pspecs[n]->name, pspecs[n]->name,
G_OBJECT_CLASS_TYPE (class), G_OBJECT_CLASS_TYPE (class),
TRUE); TRUE);