Bug 89890

Summary:	Improve error handling when systemwide dbus is not running
Product:	upower	Reporter:	Landry Breuil <landry>
Component:	general	Assignee:	Richard Hughes <richard>
Status:	RESOLVED NOTABUG	QA Contact:
Severity:	normal
Priority:	medium	CC:	ajacoutot, bugzilla
Version:	unspecified
Hardware:	Other
OS:	OpenBSD
Whiteboard:
i915 platform:		i915 features:
Attachments:	Add checks for client->priv->proxy validity

Description Landry Breuil 2015-04-03 12:01:54 UTC

Created attachment 114850 [details]
Add checks for client->priv->proxy validity

In some configurations, the systemwide dbus daemon is not running because it's not needed (i know it's mandatory on linux with all the systemd stuff, but it's not on other systems).

In that case, upower and the applications relying on it via libupower-glib just crash horribly at startup (see for example https://bugzilla.xfce.org/show_bug.cgi?id=11773). This is caused by the fact that nothing checks for client->priv->proxy validity before using it in up-client.c.

Adding fallback checks improves the situation and allows me to start an xfce4 session without system dbus running, and also allows upower -d to "gracefully" handle a not-running upowerd daemon:

(upower:25294): libupower-glib-WARNING **: Couldn't connect to proxy: Impossible de se connecter : Fichier ou répertoire introuvable

(upower:25294): libupower-glib-CRITICAL **: up_device_get_object_path: assertion 'UP_IS_DEVICE (device)' failed
Device: (null)

(upower:25294): libupower-glib-CRITICAL **: up_device_to_text: assertion 'UP_IS_DEVICE (device)' failed
(null)

(upower:25294): GLib-GObject-CRITICAL **: g_object_unref: assertion 'G_IS_OBJECT (object)' failed
Daemon:
  daemon-version:  (null)
  on-battery:      no
  lid-is-closed:   no
  lid-is-present:  no
  critical-action: (null)

Without those checks it'd just segfault.

Comment 1 Bastien Nocera 2015-04-03 13:03:06 UTC

Doesn't 0d64bbddaa0078ef148d609a3cfad854cf00d7de fix this already?

We would actually use g_return_val_if_fail() for those cases though.

Comment 2 Landry Breuil 2015-04-03 13:12:22 UTC

(In reply to Bastien Nocera from comment #1)
> Doesn't 0d64bbddaa0078ef148d609a3cfad854cf00d7de fix this already?

Nope, i was seeing the crashes with 0.99.2 which include it. It only takes care of get_property, while it can also crash on all the other codepaths (lid_is_closed/present, get_devices...) - see my patch.

> 
> We would actually use g_return_val_if_fail() for those cases though.

That's what i had at the beginning, but g_return_val_if_fail() seems to be for a "this situation shouldnt happen, let's warn out loud" case while having dbus not running should be acceptable imo. And this functions/macros can be disabled/nulled at compile time....

Your call, as long as more fallback/error codepaths are properly handled.

Comment 3 Landry Breuil 2015-04-12 20:10:58 UTC

Fwiw, here's the patch we're shipping in OpenBSD:

http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/sysutils/upower/patches/patch-libupower-glib_up-client_c?rev=1.3&content-type=text/x-cvsweb-markup

would that be acceptable as a git patch ?

Comment 4 Bastien Nocera 2015-04-13 08:58:02 UTC

(In reply to Landry Breuil from comment #2)
> [...] while having dbus not running should be acceptable imo.

Right. I'll let Richard decide about that. IMO, in 2015, it's not.

Comment 5 Landry Breuil 2015-04-13 11:25:26 UTC

(In reply to Bastien Nocera from comment #4)
> (In reply to Landry Breuil from comment #2)
> > [...] while having dbus not running should be acceptable imo.
> 
> Right. I'll let Richard decide about that. IMO, in 2015, it's not.

Well, on linux, because of systemd supremacy, i can understand it's mandatory. On other operating systems, i dont see a valid reason for that. A fullblown desktop environment like Xfce runs fine only with the session bus... if you dont use avahi/colord/polkit/etc you dont need the system bus.

Comment 6 Bastien Nocera 2017-09-08 15:35:55 UTC

(In reply to Bastien Nocera from comment #4)
> (In reply to Landry Breuil from comment #2)
> > [...] while having dbus not running should be acceptable imo.
> 
> Right. I'll let Richard decide about that. IMO, in 2015, it's not.

And it's 2017, it's even less.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.