storaged-project / udisks

The UDisks project provides a daemon, tools and libraries to access and manipulate disks, storage devices and technologies.
https://storaged.org/doc/udisks2-api/latest/
Other
348 stars 142 forks source link

Clean up GLib-GObject-WARNINGs #122

Closed tasleson closed 4 years ago

tasleson commented 8 years ago

I'm seeing a ton of these warning when running:

(lt-udisksd:25714): GLib-GObject-WARNING **: g_object_notify: object class 'UDisksLinuxBlockObject' has no property named 'block-lvm2'
(lt-udisksd:25714): GLib-GObject-WARNING **: g_object_notify: object class 'UDisksLinuxBlockObject' has no property named 'physical-volume'
(lt-udisksd:25714): GLib-GObject-WARNING **: g_object_notify: object class 'UDisksLinuxLogicalVolumeObject' has no property named 'logical-volume'
(lt-udisksd:25714): GLib-GObject-WARNING **: g_object_notify: object class 'UDisksLinuxVolumeGroupObject' has no property named 'volume-group'
(lt-udisksd:25714): GLib-GObject-WARNING **: g_object_notify: object class 'UDisksObjectSkeleton' has no property named 'manager-bcache'
(lt-udisksd:25714): GLib-GObject-WARNING **: g_object_notify: object class 'UDisksObjectSkeleton' has no property named 'manager-btrfs'
(lt-udisksd:25714): GLib-GObject-WARNING **: g_object_notify: object class 'UDisksObjectSkeleton' has no property named 'manager-iscsi-initiator'
(lt-udisksd:25714): GLib-GObject-WARNING **: g_object_notify: object class 'UDisksObjectSkeleton' has no property named 'manager-lsm'
(lt-udisksd:25714): GLib-GObject-WARNING **: g_object_notify: object class 'UDisksObjectSkeleton' has no property named 'manager-lvm2'
(lt-udisksd:25714): GLib-GObject-WARNING **: g_object_notify: object class 'UDisksObjectSkeleton' has no property named 'manager-zram'
...

It would be good to fix these so we can run with G_DEBUG=fatal-warnings

tbzatek commented 5 years ago

This is related to modules and the way how we attach extra interfaces to published d-bus objects. Purely cosmetical issue as verified during my last memleak evaluation.

This can be solved either by creating extra properties on the parent object (nasty) or by tweaking the generated GDBus code for module interfaces (less nasty, still nasty though).

tbzatek commented 5 years ago

This is caused by the default handlers of GDBusObject interface-added and interface-removed signals as generated by gdbus-codegen, triggered by g_dbus_object_skeleton_add_interface(). The most clean way would be to modify the handlers and check for presence of the particular property.

The change would look this way:

--- udisks-generated.c  2019-06-07 15:31:54.253151447 +0200
+++ udisks-generated.c.bak  2019-06-07 15:34:56.904493758 +0200
@@ -36952,7 +36952,8 @@
    * anything about, for example old generated code in this process talking to
    * newer generated code in the other process. */
   if (info != NULL)
-    g_object_notify (G_OBJECT (object), info->hyphen_name);
+    if (g_object_interface_find_property (UDISKS_OBJECT_GET_IFACE (object), info->hyphen_name))
+      g_object_notify (G_OBJECT (object), info->hyphen_name);
 }

 /**

However this would mean modifying generated code and while gdbus-codegen doesn't change that often, there's still a risk this will break in the future.

tbzatek commented 5 years ago

My other attempt to fix this without modifying the generated code turned out to be a dead end. I've tried to intercept and stop emission of the GDBusObject::interface-added and ::interface-removed signals in case of module interfaces, by calling g_signal_stop_emission_by_name(), preventing other handlers to run. However it seems there are several handlers connected and some of them need to be run to ensure correct GDBus functionality.

I've also tried retrieve GDBusObject interface from newly constructed objects and overload the interface-added and interface-removed methods. That's not a clean solution either and modifying class handlers propagate globally, leading to potentially unwanted results.

tbzatek commented 5 years ago

One other wild idea would be to somehow merge (incl. all annotations and comments) all the interface definition XML files into one during build. However since we ship udisks-generated.h as a part of public API, we'd need to generate two set of files.

vpodzime commented 5 years ago

However since we ship udisks-generated.h as a part of public API, we'd need to generate two set of files.

Why would we need two sets of files?

tbzatek commented 5 years ago

Note to myself: merged interfaces will make modules/iscsi/udisksiscsidbusutil.c and modules/lvm2/udiskslvm2dbusutil.c obsolete. One Coverity warning less.