fix: session and lock-screen reliability (screencopy freeze, dropped lock requests, logind session lookup)

[nocstah]

Summary

Three independent fixes to session/lock reliability, found while running Noctalia as my daily shell on Fedora + Hyprland. Each is a separate commit and they can be reviewed (or cherry-picked) independently:

1. fix(capture) — bound the blocking screencopy wait so a frame that never completes cannot freeze the shell.
2. fix(lockscreen) — log lock requests that are dropped because a lock is already active.
3. fix(logind) — resolve the logind session via the user's Display session when the PID lookup fails.

Updated: the PAM fork-deadlock fix that was originally the first commit here has been split into #3908, since it closes #3848 and deserves to be reviewed on its own. This PR no longer contains it. The two branches touch no files in common and can be merged in either order.

Motivation

1. Blocking screencopy can spin forever

captureOutputBlocking() looped wl_display_roundtrip() with no deadline. When the compositor never delivers ready/failed for a frame — which I reproduced with lock-screen desktop snapshots on Hyprland, on both outputs, every time — the loop spins forever inside LockScreen::lock() on the main loop. The whole shell freezes, IPC goes dark, and the session lock is never even requested, so the screen stays unlocked.

Now bounded to 2 seconds, cancelling the in-flight capture on timeout (the completion callback holds stack references), letting the caller fall back to the wallpaper background.

2. Silently ignored lock requests

lock() returned true silently when isActive(). A stale m_locked flag — which I saw after an external locker plus a compositor lock-restore sequence — then made every subsequent lock request look successful while no-op'ing. Logging the state makes the condition diagnosable; a shell restart clears it.

3. logind session resolution under the systemd user manager

Both session lookups assumed the shell runs inside the login session's cgroup. Started from the systemd user manager it does not: user@.service sits outside that cgroup, so GetSessionByPID answers NoSessionForPID, and XDG_SESSION_ID is not in that manager's environment either.

The whole logind integration was dark as a result: the session lock monitor logged disabled: session path unavailable, so loginctl lock-session never reached the lock screen; the idle-inhibit monitor never armed; and brightness fell back to the auto session path, which resolves against the caller and is the same dead end. Asking logind for this user's Display session as a last resort fixes all three, which is why brightness_service.cpp carries the same fallback in that commit.

Type of Change

• Bug fix
• New feature
• Breaking change
• Refactoring
• Build / packaging

Related Issue

None directly. Companion PR: #3908 (PAM fork deadlock, closes #3848).

Testing

• just build release — clean, no new warnings, with these three commits applied directly on top of main after the split.
• clang-format v22.1.8 run over every touched file.
• Running as my daily shell since 2026-08-10 on Fedora 44 / Hyprland, through many lock/unlock and suspend/resume cycles. The screencopy freeze reproduced on every lock attempt beforehand and none since. The logind fix is verifiable from the log line: logind session lock monitor active (/org/freedesktop/login1/session/_33) where it previously read disabled: session path unavailable.

Manual Coverage

• Tested on Niri
• Tested on Hyprland
• Tested on Sway
• Tested on another compositor:
• Tested with different bar positions and density settings
• Tested at different interface scaling values
• Tested with multiple monitors

Single machine only — MacBookPro15,1, Fedora 44, Hyprland, two external 4K outputs plus the internal panel. I have no Niri or Sway install to verify against. The capture fix is a deadline on an existing wait and is compositor-independent in nature; the logind fix is specific to being started from the systemd user manager rather than to any compositor.

Screenshots / Videos

Not applicable — no UI, visual, or layout changes.

Checklist

• This PR is ready for review, or it is marked as Draft.
• I read and followed the relevant guidance in CONTRIBUTING.md.
• I ran just format with clang-format v22+ installed, or this PR has no code changes.
• I ran the relevant build or test commands, or explained why they were not run.
• I self-reviewed the changes.
• I checked for new warnings or errors.
• I will update end-user documentation after merge, or this PR does not change user-facing configuration or behavior.
• I added or updated assets/translations/en.json, or this PR adds no new user-facing strings.
• I did not edit non-English translation files unless this PR is explicitly for translation tooling, an import/export sync, or a maintainer-requested locale change.
• I used the existing canonical names for config keys, IPC names, paths, and identifiers.

Additional Notes

The 2-second screencopy deadline is a chosen constant. If you would rather it were configurable, or a different default, that is an easy change.

Happy to split these three further if you would prefer one PR per fix.

[decksters-lab]

Im excited for this!

[nocstah]

Split the PAM fork-deadlock commit out into #3908 — it closes #3848 and is easier to review on its own. This PR is now the remaining three fixes (screencopy timeout, dropped-lock-request logging, logind session fallback) and no longer touches src/auth/ or src/main.cpp. Both branches build cleanly on top of main independently and share no files, so they can be merged in either order.

[Ly-sec]

Thanks for splitting these fixes out @nocstah . The lock-request logging change looks good, and the logind session-resolution change is a useful direction.

I would request changes on the screencopy timeout before merging. The deadline is checked only after wl_display_roundtrip() returns. If that call itself blocks because the Wayland connection stops making progress, the timeout cannot fire. Repeated blocking roundtrips with sleep_for() also do not fit our poll-based event-loop model.

Could this use a deadline-aware wait on the Wayland file descriptor instead, then dispatch events while progress is available? Please also add coverage for the timeout and cancellation path, including that a late completion callback cannot access the stack references captured by captureOutputBlocking().

For the logind fallback, please extract the duplicated session-resolution logic into logind-owned shared code. User.Display is a sensible best-effort path for a shell started by the systemd user manager, but it may select a different session when a user has multiple concurrent sessions, so that behavior should be explicit and tested.

[nocstah]

Thanks for the review — all three addressed in the two follow-up commits.

The deadline is checked only after wl_display_roundtrip() returns…

Fixed — the roundtrip is gone. captureOutputBlocking() now waits on the Wayland fd directly: dispatch_pending → prepare_read → flush → poll(fd, remaining) → read_events → dispatch_pending. The wait is bounded by what's left of the budget rather than being a blocking call with a deadline checked around it, and there's no sleep_for — each pass either makes progress or returns when its slice of the deadline expires.

Please also add coverage for the timeout and cancellation path…

tests/screencopy_blocking_test.cpp. The capture and wait sides moved into screencopy_blocking.{h,cpp} behind small injectable ops, so the tests drive a virtual clock with no compositor: synchronous completion, completion after N pumps, timeout with cancellation, no single wait exceeding the remaining budget, one wait consuming the whole budget, and dispatch failure.

The last case is the one you asked about specifically: completion state now lives in a shared_ptr that gets flagged abandoned when we give up, so the callback captures nothing from the caller's stack. The test fires a completion after runBlockingCapture() has returned and asserts the caller's frame and error are untouched.

For the logind fallback, please extract the duplicated session-resolution logic into logind-owned shared code.

Done — src/dbus/logind/logind_session.{h,cpp}. Both LogindService and BrightnessService call logind::resolveSession(), which returns the path plus the lookup that answered (XDG_SESSION_ID, pid, or user Display); both log that source. BrightnessService keeps its auto path only for when nothing resolves.

…it may select a different session when a user has multiple concurrent sessions, so that behavior should be explicit and tested.

Agreed. The resolver now reads the user's full Sessions list and, when there's more than one, warns naming the session it guessed instead of returning it silently:

[WRN] [logind] logind: 2 concurrent sessions for this user, falling back to the
      Display session 3 (/org/freedesktop/login1/session/_33) — it may not be the
      session this process runs in

The lookups are injectable, so tests/logind_session_test.cpp covers the order (id wins, no fallback consulted), a stale XDG_SESSION_ID falling through to the pid, the systemd-user-manager case, the multi-session guess, no display session, and every lookup dark.

Left as two follow-up commits so the review diff stays readable — happy to squash into the originals if you'd prefer.