-
Notifications
You must be signed in to change notification settings - Fork 4
Start singularity-session.target so graphical-session.target actually activates #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
mirkobrombin
merged 6 commits into
singularityos-lab:main
from
perlowja:fix/graphical-session-target
Sep 17, 2026
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
d02aa35
session: start singularity-session.target so graphical-session.target…
perlowja fcc386a
fix: install session target via systemd's own user-unit dir
perlowja 4817210
fix: trim session-target comments to the non-obvious facts
perlowja b9ef1df
session: stop singularity-session.target when the session ends
perlowja 702451d
fix: point session-target-lifecycle test at the built launcher
perlowja 57fe5c2
session: reference-count singularity-session.target stop
perlowja File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| [Unit] | ||
| Description=Singularity Desktop session | ||
| BindsTo=graphical-session.target | ||
| Wants=graphical-session-pre.target | ||
| After=graphical-session-pre.target |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| #!/usr/bin/env bash | ||
| # singularity-session.target is started by the desktop-session launcher and | ||
| # stopped by nothing else. graphical-session.target is StopWhenUnneeded=yes, so | ||
| # it only goes away once nothing binds it -- and a user manager that outlives | ||
| # the session (lingering enabled, or a second concurrent login) keeps ours | ||
| # active after the compositor exits, and with it the portal backend and foot | ||
| # server underneath. This asserts the launcher's exit path issues the matching | ||
| # stop, and that the older pid-file cleanup still runs alongside it. | ||
| set -eu | ||
|
|
||
| # Re-entrant stub: the launcher runs this same file as singularity-desktop, and | ||
| # a non-zero exit drives the supervisor to its crash budget so the script exits. | ||
| if [ "${SINGULARITY_TEST_FAKE_DESKTOP:-0}" = "1" ]; then | ||
| exit 1 | ||
| fi | ||
|
|
||
| TEST_DIR=$(mktemp -d "${TMPDIR:-/tmp}/singularity-session-target-test.XXXXXX") | ||
| trap 'rm -rf "$TEST_DIR"' EXIT | ||
| mkdir -p "$TEST_DIR/state" "$TEST_DIR/runtime" "$TEST_DIR/bin" | ||
| chmod 700 "$TEST_DIR/runtime" | ||
|
|
||
| SYSTEMCTL_LOG="$TEST_DIR/systemctl.log" | ||
| : > "$SYSTEMCTL_LOG" | ||
|
|
||
| cat > "$TEST_DIR/bin/systemctl" <<EOF | ||
| #!/bin/sh | ||
| printf '%s\n' "\$*" >> "$SYSTEMCTL_LOG" | ||
| EOF | ||
| # Keep the launcher's environment probing off the machine running the test. | ||
| for stub in pkill xdg-user-dirs-update dbus-update-activation-environment; do | ||
| printf '#!/bin/sh\nexit 0\n' > "$TEST_DIR/bin/$stub" | ||
| done | ||
| printf '#!/bin/sh\nprintf "false\\n"\n' > "$TEST_DIR/bin/gsettings" | ||
| chmod +x "$TEST_DIR"/bin/* | ||
|
|
||
| export PATH="$TEST_DIR/bin:$PATH" | ||
| export XDG_STATE_HOME="$TEST_DIR/state" | ||
| export XDG_RUNTIME_DIR="$TEST_DIR/runtime" | ||
| export DBUS_SESSION_BUS_ADDRESS="test-bus" | ||
| export SINGULARITY_SESSION_BUILD_ID="session-target-test-build" | ||
| export SINGULARITY_DESKTOP_BINARY="$0" | ||
| export SINGULARITY_TEST_FAKE_DESKTOP=1 | ||
|
|
||
| # The launcher under test is generated, so point the test at the configured | ||
| # copy in the build dir (SINGULARITY_TEST_DESKTOP_SESSION, set in meson.build). | ||
| # Outside meson, fall back to the raw .in template. | ||
| LAUNCHER="${SINGULARITY_TEST_DESKTOP_SESSION:-$(dirname "$0")/../src/singularity-desktop-session.in}" | ||
| # The supervisor kills $PPID once it gives up, so run the launcher under a | ||
| # wrapper that absorbs that signal instead of the test process itself. | ||
| set +e | ||
| bash -c 'trap "" TERM; bash "$1" & child=$!; wait "$child"' _ "$LAUNCHER" | ||
| set -e | ||
|
|
||
| grep -Fq -- "--user --no-block start singularity-session.target" "$SYSTEMCTL_LOG" || { | ||
| echo "launcher never started singularity-session.target" >&2 | ||
| cat "$SYSTEMCTL_LOG" >&2 | ||
| exit 1 | ||
| } | ||
| grep -Fq -- "--user --no-block stop singularity-session.target" "$SYSTEMCTL_LOG" || { | ||
| echo "launcher exited without stopping singularity-session.target" >&2 | ||
| cat "$SYSTEMCTL_LOG" >&2 | ||
| exit 1 | ||
| } | ||
|
|
||
| # Order matters: a stop that raced ahead of the start would leave the target up. | ||
| START_LINE=$(grep -Fn -- "start singularity-session.target" "$SYSTEMCTL_LOG" | head -1 | cut -d: -f1) | ||
| STOP_LINE=$(grep -Fn -- "stop singularity-session.target" "$SYSTEMCTL_LOG" | head -1 | cut -d: -f1) | ||
| [ "$STOP_LINE" -gt "$START_LINE" ] || { | ||
| echo "stop (line $STOP_LINE) did not follow start (line $START_LINE)" >&2 | ||
| cat "$SYSTEMCTL_LOG" >&2 | ||
| exit 1 | ||
| } | ||
|
|
||
| # The pre-existing pid-file cleanup has to survive being moved into the | ||
| # trap function that now also stops the target. | ||
| [ ! -e "$XDG_RUNTIME_DIR/singularity-desktop-session.pid" ] || { | ||
| echo "launcher left its pid file behind" >&2 | ||
| exit 1 | ||
| } | ||
|
|
||
| # Scenario 2: a sibling login (same UID, same XDG_RUNTIME_DIR, same systemd | ||
| # --user manager) is still running when this login exits. Simulated by | ||
| # planting a session marker the launcher did not create itself -- it must | ||
| # leave the target running for the sibling rather than stopping it. | ||
| : > "$SYSTEMCTL_LOG" | ||
| SIBLING_MARKER="$XDG_RUNTIME_DIR/singularity-session.d/sibling-fake-pid" | ||
| mkdir -p "$(dirname "$SIBLING_MARKER")" | ||
| : > "$SIBLING_MARKER" | ||
|
|
||
| set +e | ||
| bash -c 'trap "" TERM; bash "$1" & child=$!; wait "$child"' _ "$LAUNCHER" | ||
| set -e | ||
|
|
||
| grep -Fq -- "--user --no-block start singularity-session.target" "$SYSTEMCTL_LOG" || { | ||
| echo "launcher never started singularity-session.target on the second run" >&2 | ||
| cat "$SYSTEMCTL_LOG" >&2 | ||
| exit 1 | ||
| } | ||
| if grep -Fq -- "--user --no-block stop singularity-session.target" "$SYSTEMCTL_LOG"; then | ||
| echo "launcher stopped singularity-session.target while a sibling login's marker was still present" >&2 | ||
| cat "$SYSTEMCTL_LOG" >&2 | ||
| exit 1 | ||
| fi | ||
| [ -e "$SIBLING_MARKER" ] || { | ||
| echo "launcher removed a marker it did not create" >&2 | ||
| exit 1 | ||
| } | ||
| [ ! -e "$XDG_RUNTIME_DIR/singularity-desktop-session.pid" ] || { | ||
| echo "launcher left its pid file behind on the second run" >&2 | ||
| exit 1 | ||
| } |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the user manager survives logout—such as with lingering enabled or another concurrent login—this start has no matching stop, so
singularity-session.targetremains active after labwc terminates. Because it keeps theStopWhenUnneeded=yesgraphical target needed, graphical-session services such as the portal backend and foot server continue running outside the desktop session; add teardown tied to the launcher/compositor lifetime.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in aec8a46.
The pid-file cleanup that was already on
trap ... EXITbecomes_session_cleanup(), and the matchingsystemctl --user --no-block stop singularity-session.targetgoes in there alongside it.EXITonly, deliberately. bash runs the EXIT trap for an untrapped fatal signal as well, whereas an explicittrap ... TERMis deferred until the foreground command returns — and on the logout path the supervisor is blocked in"$_DESKTOP", so it never would. Checked on bash 5.3.9 (and 3.2.57): SIGTERM to the launcher while it supervised a long-lived fake shell issued the stop and removed the pid file.Added
tests/session_target_lifecycle_test.sh(wired into meson assession-target-lifecycle). It drives the launcher to its crash budget behind stubsystemctl/pkill/gsettings/xdg-user-dirs-updateand asserts the start, the stop, their ordering, and the pid-file removal. Against the pre-fix script it fails with:Both tests green on aarch64:
session-safe-mode OK 4.17s,session-target-lifecycle OK 4.34s.