GNOME Bugzilla – Bug 688076
handle shutdown with multiple sessions more gracefully
Last modified: 2012-11-23 18:21:11 UTC
Created attachment 228674 [details] [review] patch When there are multiple sessions, we may get a polkit dialog after the user confirms the power off dialog. Previously, we didn't handle that very nicely, since we were already in the EndSession phase when calling PowerOff, and there is no way back to the Running phase from there, even if the user cancels the polkit dialog. This commit rearranges things so that we call PowerOff before leaving the QueryEndSession phase. To prevent the system from going down right away, we take a delay inhibitor, and listen for the PrepareForShutdown signal to know when we can safely transition into EndSession. If the user cancels the polkit dialog, the PowerOff call fails with an AccessDenied error, which we handle and return to the Running phase. All of this only works with systemd. Shutdown with ConsoleKit still works the same as before.
Still to do: Also show user sessions in the shell poweroff dialog, similar to session inhibitors.
Created attachment 229696 [details] [review] patch Newer version of the same patch, which implements the same prepare/complete shutdown api for ConsoleKit as well (prepare is a no-op there, and complete does all the work).
Review of attachment 229696 [details] [review]: Hm, this is all pretty complicated. Not your fault, of course; really the gnome-session internals weren't designed with this idea that we might have to authenticate shutdown. On the systemd side, I wonder if it's actually the expected/guaranteed semantics that authorization will be requested even when a delay inhibitor is taken. I guess it kind of has to work that way, since otherwise systemd wouldn't know whether it's actually going to perform the operation that's delayed. It's just weird that API users have to take out a delay lock on the operation they're requesting. Oh well; two minor comments, feel free to commit after addressing: ::: gnome-session/gsm-manager.c @@ +4164,3 @@ + end_phase (manager); + } + else { Should be } else { ::: gnome-session/gsm-systemd.c @@ +780,3 @@ + + if (result == NULL) { + g_warning ("shutdown failed: %s", error->message); If we get an error here because the user failed to authorize the shutdown, it'd be nice to not pollute the logs with a g_warning(). if (!g_error_matches (error, ...)) ?
(In reply to comment #3) > Review of attachment 229696 [details] [review]: > > Hm, this is all pretty complicated. Not your fault, of course; really the > gnome-session internals weren't designed with this idea that we might have to > authenticate shutdown. This is already a slimmed down version. My original CK scheme for coordinated shutdown was even more complicated... it featured informing all running sessions about the shutdown request, and giving them a chance to inhibit it. https://bugs.freedesktop.org/show_bug.cgi?id=24493 if you are curious. > On the systemd side, I wonder if it's actually the expected/guaranteed > semantics that authorization will be requested even when a delay inhibitor is > taken. I guess it kind of has to work that way, since otherwise systemd > wouldn't know whether it's actually going to perform the operation that's > delayed. I talked to Lennart about this - he told me that systemd is checking privileges before looking for delay inhibitors. > It's just weird that API users have to take out a delay lock on the operation > they're requesting. > > Oh well; two minor comments, feel free to commit after addressing: Sure, will address those - I need to wait for the blocking gnome-shell bug before committing this. And I want to do some testing too.