GNOME Bugzilla – Bug 672240
GDM crashes when systemd is installed but not in use
Last modified: 2012-05-08 20:55:47 UTC
Created attachment 209939 [details] [review] patch for issue If systemd is installed but not in use, gdm gnome-shell crashes: JS ERROR: !!! Exception was: Error: Error invoking Gio.init: Error calling StartServiceByName for org.freedesktop.login1: GDBus.Error:org.freedesktop.DBus.Error.Spawn.ChildExited: Launch helper exited with unknown return code 1 JS ERROR: !!! lineNumber = '0' JS ERROR: !!! fileName = '"gjs_throw"' JS ERROR: !!! stack = '"("Error invoking Gio.init: Error calling StartServiceByName for org.freedesktop.login1: GDBus.Error:org.freedesktop.DBus.Error.Spawn.ChildExited: Launch helper exited with unknown return code 1")@gjs_throw:0 (null)@/usr/share/gjs-1.0/overrides/Gio.js:242 ([object _private_Gio_DBusConnection],"org.freedesktop.login1","/org/freedesktop/login1")@/usr/share/gjs-1.0/overrides/Gio.js:201 SystemdLoginManager()@/usr/share/gnome-shell/js/gdm/systemd.js:26 ()@/usr/share/gnome-shell/js/gdm/powerMenu.js:37 wrapper()@/usr/share/gjs-1.0/lang.js:204 ()@/usr/share/gjs-1.0/lang.js:145 ()@/usr/share/gjs-1.0/lang.js:239 ()@/usr/share/gnome-shell/js/ui/panel.js:1113 wrapper()@/usr/share/gjs-1.0/lang.js:204 start()@/usr/share/gnome-shell/js/ui/main.js:219 @<main>:1tg b "' The attached patch should fix the issue.
Review of attachment 209939 [details] [review]: Looks good. Please commit!
Created attachment 209964 [details] [review] powerMenu: Refactor systemd/consolekit code Rather than having two very similar implementations inline, stick in a "PowerMenuBackend" interface that's implemented by both Systemd and ConsoleKit, in a separate file. This removes a lot of tricky branching code paths under the name of consolidation. There's this as well... Completely untested (because I can't figure out how to test GDM), so feedback would be helpful.
Review of attachment 209964 [details] [review]: Certainly the abstraction cleans up the caller code and isn't a bad idea overall. I don't think we should do it before hard code freeze. Instead we should go with the more surgical patch already mentioned. Clean ups post release make sense, I think, though. ::: js/gdm/powerMenu.js @@ +33,3 @@ _init: function() { this.parent('system-shutdown', null); + this._powerMenuBackend = new Systemd.SystemdLoginManager(); shouldn't this say PowerMenuBackend? ::: js/gdm/powerMenuBackend.js @@ +58,3 @@ + let implMethod = mapping[apiMethod]; + self[apiMethod + 'Sync'] = self[implMethod + 'Sync']; + self[apiMethod + 'Remote'] = self[implMethod + 'Remote']; eww, i don't like this magic. Note The methods aren't 1-to-1 compatible, either.
Created attachment 209977 [details] [review] powerMenu: Refactor systemd/consolekit code Rather than having two very similar implementations inline, stick in a "PowerMenuBackend" interface that's implemented by both Systemd and ConsoleKit, in a separate file. This removes a lot of tricky branching code paths under the name of consolidation. ugh, this "refactor" is starting to seem less and less worthwhile. Talking on IRC about Deferreds, well, this: this._loginManager.CanRebootRemote(function(result, error) { callback((result != 'no'), error); }); would become: let d = this._loginManager.CanRebootRemote(); d.addCallback(function(result) { return (result != 'no'); }); return d; That is, callback chaining, where the result of one callback gets sent to the next, allowing you to normalize values without the client suspecting it. It would do: let d = this._powerMenuBackend.CanReboot(); d.addCallback(function(result) { this._haveSuspend = result; this._updateVisibility(); }); d.addErrback(function(failure) { this._haveSuspend = false; this._updateVisibility(); }); By transforming the two-way call stack into a one-way call stack, things get much simpler, especially with tricks like callback chaining. Again, I wish we had this in GLib/Gio itself. But that's not the task at hand, here...
Review of attachment 209939 [details] [review]: Pushed this one.