SouveraineOS/docs/tasks/48-lock-surface-reload-fatal.md

TASK 48 — The shell dies on any scene reload that owes a lock

Status: unblocked, still unproved, 2026-08-17. The daemon half remains live. Viewtop's admission and restore state machine has since landed and its two-client lock handoff was proved end to end, so TASK-43 is no longer a prerequisite. The QML acceptance below has not been rerun; the current root Scope still has no reloadableId even though its continuity object and WlSessionLock children do.

• Daemon half: done and live. sessiond admits a same-pid re-registration; tested in CI (4 tests) and installed on the phone as 0.1.r298.gb5ee87da0fe1. The "refused, retrying forever" deadlock is gone.
• Shell half: shipped and DOES NOT WORK. Measured on the phone at 00:14 by appending a comment to the composed LockScreen.qml while the session was locked: crash directories went 13 → 14 with FATAL: Tried to show lockscreen surfaces without active lock. The PersistentProperties carry does not survive the reload — its onLoaded logged nothing, so held came back false and the adopt branch was never taken. Do not trust the fix; the acceptance run below still fails at step 1.

The reason is object matching, and it is the thing to attack next. Reloadable::reloadRecursive only hands a new object its predecessor when the object has a non-empty reloadableId and a non-null oldRoot, or when its parent is a ReloadPropagator matching children positionally. LockScreen.qml's own root Scope carries no reloadableId, so if anything in the chain from ShellRoot down to Lock.qml breaks the propagation, every child of that Scope reloads with oldInstance == nullptr — which is what the measurement shows. Find where the chain breaks before writing more QML.

There is also a simpler guard worth taking regardless, because it converts the crash into the safe failure the constraints already demand: after reload, lock.secure && !lock.locked means this process holds a session lock that this instance did not adopt, and requesting one from that state is guaranteed to hit the qFatal. secure reads the process-global manager, so it is true even for a fresh unmatched instance — it is exactly the signal needed, and gating the request on it fails closed (stays locked, no surface) instead of crashing. That is not a fix; it is the difference between a crash and a black screen, and doctrine already says which one it prefers. Size: one session, but it is on the lock path — read TASK-43 first. Repo: souveraine (surfaces/quickshell/modules/common/panels/lock/LockScreen.qml, src/sessiond/server.rs).

Symptom

FATAL: Tried to show lockscreen surfaces without active lock
ERROR: Quickshell has crashed under pid <n>
ERROR: Quickshell has been restarted.

Eleven crash directories under ~/.cache/quickshell/crashes/ going back to 2026-07-17, so this is long-standing and roughly every second or third day. Two on 2026-07-29 alone (09:14:37, 10:41:11), both immediately after a scene reload.

Mechanism

LockScreen.qml:104 is

WlSessionLock {
    locked: GlobalStates.screenLocked
    surface: root.sessionLockSurface
}

On a scene reload the whole QML tree is rebuilt while the outgoing instance is still alive and still holds the compositor's ext-session-lock. In the new tree GlobalStates.screenLocked starts false. Then SessiondBridge registers, sessiond answers must_lock=true (the session is locked), and registerWithAuthority() sets GlobalStates.screenLocked = true.

The binding fires, quickshell asks for the lock, the compositor denies it — another client holds it — and the surface component is instantiated anyway. That is the FATAL.

The file already knows about this denial path. onLockStateChanged carries a comment about ext-session-lock finished "denied because another client held it", and resyncs screenLocked when it happens. But that handler runs after surfaces were attempted, so the abort beats the guard to it every time.

Corrected 2026-07-31, by reading quickshell's session_lock.cpp

The above is right about the trigger and wrong about the consequence, and the missing half is worse than the crash. WlSessionLock::onReload adopts the outgoing instance's SessionLockManager, then calls realizeLockTarget() with whatever locked evaluated to at construction. GlobalStates is a fresh singleton by then, so screenLocked is false, so lockTarget is false — and the false branch is unlock(), which on an adopted manager that is locked sends ext_session_lock_v1.unlock_and_destroy.

So a scene reload does not merely fail to raise a surface. It unlocks the session. What is left is a compositor with no lock surface, a screen that reads black because the idle budget blanked it, and {"locked":false,"lockRequested":true} from device_state. No crash, no FATAL, nothing in the crash directory. That is the third variant seen on the phone on 2026-07-30 and 07-31, and it is why "eleven crash directories" was always an undercount of how often this fires.

The FATAL is the other branch of the same root cause: when the adoption does not match, the new manager's lock() is refused — SessionLockManager::lock() returns false if sessionLocked(), and the outgoing lock is still the process-global holder — after which realizeLockTarget calls updateSurfaces(true) anyway. One cause, two faces, and which one you get depends only on whether the reloader matched the old object.

The fix

GlobalStates.screenLocked must be true at the moment the new tree's WlSessionLock is constructed, which makes realizeLockTarget take the adopt branch: surfaces are recreated against the same compositor lock, manager->lock() declines harmlessly because we already hold it, and updateSurfaces sees an active lock. No re-request, no denial, no unlock_and_destroy. This is the "adopting beats re-requesting" constraint below, and it is the only branch that never opens the panel.

The request is carried across the reload by PersistentProperties (in-process and synchronous — Persistent is a file and answers too late to be read at construction), declared before the WlSessionLock because Scope is a ReloadPropagator and reloads its children in declaration order.

The daemon owed the other half. The authority lease is per-connection, but the shell is a process: an in-process reload opens a second connection while the first is still open, so shell_ready was refused as "already registered" forever. sessiond now admits a registration whose SO_PEERCRED pid equals the lease holder's — the kernel's answer, not a mirrored bool — and bumps heartbeat_gen, which already existed to make the superseded connection's later EOF inert. Anything that cannot be proven to be the same process is still refused, so the exclusive lease keeps every case it was written for.

Ordering, and it is TASK-28's case again: the QML must reach the device before the daemon. New sessiond with old QML admits the reload, answers must_lock=true, and the old tree re-requests a lock it cannot have — the FATAL. Old sessiond with new QML merely keeps the stale lease, with the surface adopted and visible. The QML went first, deliberately.

Why it surfaced now

It did not — it was always there. It was partly masked: before souveraine 45fbbea, a reload's shell_ready was refused ("already registered") and the shell gave up permanently, so must_lock was never applied and screenLocked was never set. The reload stayed silently unregistered instead of crashing. Fixing the registration made the reload reach the lock path, which is correct behaviour meeting a real bug.

So: the fix for TASK-48 is not to undo 45fbbea. An unregistered shell is strictly worse — sessiond believed there was no shell for 90 minutes.

What the fix has to respect

• Do not gate the surface on secure. secure is the compositor's ack that a surface is up; using it as the precondition for putting one up is circular.
• Adopting beats re-requesting. The phone and laptop both run misc:allow_session_lock_restore = true, which is what lets a relaunched qs -c souveraine take over an abandoned lock. A reload should land on that same path rather than racing a second acquire against a live holder.
• Never open the panel to close the race. Failing closed here means staying locked and crashing loudly, which is what it does today. Any fix that trades the crash for a briefly unlocked panel is worse than the crash.
• TASK-43's note applies verbatim: the two-client lock handoff is the thing not to get wrong. viewtop's ext-session-lock admission gate is the same mechanism — pull that repo before touching this.

Acceptance

1. touch any file in the composed tree while the session is locked. The shell reloads, re-registers, sends locked_ack, and does not crash.
2. ~/.cache/quickshell/crashes/ gains no new directory across ten such reloads.
3. The panel never becomes visible during the reload — verify with grim, not by looking, since the window is under a second.
4. {"op":"device_state"} reports shell_alive: true and phase: released afterwards.

Connects to

TASK-43 (viewtop's lock admission gate — the other half of two-client lock), TASK-02 (lockscreen as a Rust system), LOCK-DPMS-LESSONS.md §1, souveraine 45fbbea (the registration fix that unmasked this).