Created attachment 303747 [details] [review]
Prevent deadlock in g_object_notify_queue_add()

If too many GParamSpecs are queued we would leave the notify_lock locked.

Created attachment 303748 [details] [review]
Use a GPtrArray for the GObjectNotifyQueue

The contents are reversed before calling dispatch_properties_changed() to replicate
the old behavior.

----

There is a test that requires the old behavior, no idea if it is actually required.

Created attachment 303749 [details] [review]
Use g_datalist_id_replace_data() for GObjectNotifyQueue

This further limits the time the notify_lock is held and allows us to avoid the lock in the conditional path.

Review of attachment 303747 [details] [review]:

You shouldn't access n_pspecs without the lock held.

I consider it basically impossible for this case to be hit, and if it ever is hit, the program is going to misbehave -- I'd rather it was just converted into an assert inside of the lock.

Review of attachment 303748 [details] [review]:

Changes like this should come with benchmark numbers...

Review of attachment 303749 [details] [review]:

::: gobject/gobject.c
@@ +234,3 @@
       nqueue->pspecs = g_ptr_array_new ();
+
+      if (!g_datalist_id_replace_data (&object->qdata, quark_notify_queue,

This approach doesn't work.

g_object_notify_queue_thaw() sets the qdata back to NULL.

If that is happening in another thread then we could see ourselves with a failure to set this (because it was not yet NULL) followed by reading out NULL below (since the other thread meanwhile changed the value).

Comment on attachment 303747 [details] [review]
Prevent deadlock in g_object_notify_queue_add()

I've changed the return-if-fail to an assertion now.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1040.