csm-systemd: wait for logind's PrepareForShutdown before quitting - #215
Open
leigh123linux wants to merge 1 commit into
Open
csm-systemd: wait for logind's PrepareForShutdown before quitting#215leigh123linux wants to merge 1 commit into
leigh123linux wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed failure-path issues that can crash initialization or leave the session stuck in a non-running phase when shutdown/reboot isn’t confirmed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves cinnamon-session’s shutdown/reboot flow when using logind by taking its own delay inhibitor, dropping conflicting block inhibitors, and waiting for logind’s PrepareForShutdown signal before quitting; it also removes the long-dead MDM display-manager fallback code paths.
Changes:
- Add a
shutdown-preparedsignal andcomplete_shutdown()hook to theCsmSysteminterface, and implement delay-inhibitor handling +PrepareForShutdownlistening in the systemd backend. - Update
CsmManagerto quit only after shutdown is confirmed (and remove the old request-failed → MDM fallback). - Remove
mdm.c/mdm.hand update the build to stop compiling MDM logout-action support.
File summaries
| File | Description |
|---|---|
| cinnamon-session/meson.build | Removes mdm.c from the build sources. |
| cinnamon-session/mdm.h | Deletes legacy MDM logout-action API header. |
| cinnamon-session/mdm.c | Deletes legacy MDM protocol implementation. |
| cinnamon-session/csm-systemd.c | Adds delay inhibitor + waits for PrepareForShutdown, emits shutdown-prepared, implements complete_shutdown(). |
| cinnamon-session/csm-system.h | Extends CsmSystemInterface with complete_shutdown(). |
| cinnamon-session/csm-system.c | Adds shutdown-prepared signal and csm_system_complete_shutdown() wrapper. |
| cinnamon-session/csm-manager.c | Removes MDM fallback and waits for shutdown-prepared before quitting. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+470
to
+474
| on_shutdown_prepared (CsmSystem *system, | ||
| gboolean success, | ||
| gpointer user_data) | ||
| { | ||
| g_warning ("Using an MDM logout action to shutdown/reboot the system."); | ||
| MdmLogoutAction fallback_action = GPOINTER_TO_INT (user_data); | ||
| mdm_set_logout_action (fallback_action); | ||
| csm_quit (); | ||
| g_signal_handlers_disconnect_by_func (system, on_shutdown_prepared, user_data); |
Comment on lines
+158
to
162
| } else { | ||
| g_signal_connect (manager->priv->sd_proxy, "g-signal", | ||
| G_CALLBACK (sd_proxy_signal_cb), manager); | ||
| } | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cinnamon-session quit immediately after requesting PowerOff/Reboot via
logind, without confirming logind had actually accepted the request. If
another process held a delay inhibitor, cinnamon-session would tear
down and hand back to the display manager greeter well before the
machine actually powered off, leaving the session in a misleading
half-terminated state for the duration of the delay.
Take our own delay inhibitor before requesting shutdown/restart, drop
any held block inhibitor (which would make logind refuse the request
outright), and wait for logind's PrepareForShutdown signal to confirm
shutdown is genuinely proceeding before quitting -- matching
gnome-session's gsm_systemd_prepare_shutdown()/complete_shutdown().
Also remove the MDM display-manager fallback path (quit_request_failed,
mdm_set_logout_action calls, the CSM_MANAGER_LOGOUT_*_MDM enum values,
and mdm.c/mdm.h) since it's dead: Linux Mint dropped MDM for LightDM
years ago, and the fallback was never actually reachable once
csm_manager_quit() started quitting unconditionally after issuing the
shutdown/restart request.
Fixes: #214