After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 731200 - unconditional 'notify' during g_object_set() is problematic
unconditional 'notify' during g_object_set() is problematic
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gobject
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-06-04 11:19 UTC by Allison Karlitskaya (desrt)
Modified: 2014-06-06 14:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gobject: add 'explicit notify' GParamSpec flag (2.45 KB, patch)
2014-06-04 13:05 UTC, Allison Karlitskaya (desrt)
committed Details | Review
test: add test for explicit-notify properties (4.23 KB, patch)
2014-06-06 14:25 UTC, Allison Karlitskaya (desrt)
none Details | Review
test: add test for explicit-notify properties (4.23 KB, patch)
2014-06-06 14:38 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Allison Karlitskaya (desrt) 2014-06-04 11:19:32 UTC
See bug 730886 for some examples of why this is an issue.

More generally, quite a lot of classes these days are implemented in a pretty consistent way, with their property set function looking like:



  switch (prop_id)
    {
    case PROP_ONE:
      my_obj_set_one (obj, g_value_get_thing ());
      break;
    case PROP_TWO:
      my_obj_set_two (obj, g_value_get_thing ());
      break;
    case etc:

ie: always calling the public accessors.

Those functions, because they are public C API, always emit "notify" in case of changes.  It is common practice, however, to avoid setting the value (and avoid the notify) in the case that the value being set is the same as what was already there.

g_object_set() always emits notify on the property in question, even in the case that the class didn't do it for itself.

We should have some mechanism by which classes can indicate to GObject that this is not desired.

A few possibilities present:

 - have a call that the user can make during a set_property invocation which
   tells GObject to skip the emission

 - have a flag that can be set on the type at class_init() time to prevent
   notify by being emitted on properties of this class

 - have a flag on the paramspec


From an implementation standpoint, a flag on the paramspec would be cleanest.  It's a bit annoying to use, however: we would be adding one more "flag that you always want to use" (eg: G_PARAM_STATIC_STRINGS).

The "skip emission" call is conceptually ugly.

The per-class flag is an interesting thought.  In fact, we could introduce a per-class GParamFlags that gets OR'd into all GParamSpec installed onto this object.  We could also just make a convention that having a

#define MY_PARAM_FLAGS  (G_PARAM_STATIC_STRINGS | G_PARAM_NO_NOTIFY)

and using that is "standard practice", but that's a bit weak...
Comment 1 Allison Karlitskaya (desrt) 2014-06-04 13:05:55 UTC
Created attachment 277872 [details] [review]
gobject: add 'explicit notify' GParamSpec flag

Add a flag to prevent the automatic emission of the "notify" signal
during g_object_set_property().

If this flag is set then the class must explicitly emit the notify
for themselves.  This is already standard practice on most classes, but
we cannot simply remove the existing behaviour because there are surely
many cases where it is needed.
Comment 2 Matthias Clasen 2014-06-04 15:14:32 UTC
Looks like a very reasonable addition to gobject.

Of course, fixing the thousands of properties where this is needed is left as an exercise for the reader :-(
Comment 3 Allison Karlitskaya (desrt) 2014-06-06 14:25:59 UTC
Created attachment 278033 [details] [review]
test: add test for explicit-notify properties
Comment 4 Allison Karlitskaya (desrt) 2014-06-06 14:38:36 UTC
Created attachment 278035 [details] [review]
test: add test for explicit-notify properties
Comment 5 Allison Karlitskaya (desrt) 2014-06-06 14:38:51 UTC
Attachment 277872 [details] pushed as bbdb234 - gobject: add 'explicit notify' GParamSpec flag
Attachment 278035 [details] pushed as 0208861 - test: add test for explicit-notify properties