diff --git a/shell/plugins/lock/Service.qml b/shell/plugins/lock/Service.qml index e6bc4ea5..c775ba02 100644 --- a/shell/plugins/lock/Service.qml +++ b/shell/plugins/lock/Service.qml @@ -57,6 +57,13 @@ Item { property bool strandedLockResolved: false readonly property bool locked: lockRequested || sessionLock.locked || sessionLock.secure + + // Decisions read the lock state afresh. A property binding that missed a change + // notification once reported locked after an unlock, and a lock request before + // suspend then took that for done and never locked the session. + function currentlyLocked() { + return lockRequested || sessionLock.locked || sessionLock.secure + } readonly property bool authenticating: authenticatingPassword || fingerprintAuthenticating readonly property var batteryService: shell && shell.services ? shell.firstPartyServiceFor("omarchy.battery") : null readonly property bool powerSaverActive: batteryService ? batteryService.powerSaverOnBattery : false @@ -109,7 +116,7 @@ Item { if (strandedLockResolved || strandedLockCheckProc.running) return // A lock this shell took is nobody's orphan. - if (locked || lockRequested) { + if (currentlyLocked()) { strandedLockResolved = true return } @@ -118,7 +125,7 @@ Item { } function recoverStrandedLock() { - if (!strandedLock || locked || !passwordPamConfigured) return + if (!strandedLock || currentlyLocked() || !passwordPamConfigured) return strandedLock = false logEvent("lock-stranded: recovering") @@ -218,7 +225,7 @@ Item { } function finishUnlock() { - if (!root.locked && !lockRequested) return + if (!root.currentlyLocked()) return lockRequested = false pendingSessionLock = false @@ -688,7 +695,7 @@ Item { root.strandedLockResolved = true // A lock taken while this was in flight is this shell's own. - root.strandedLock = exitCode === 0 && !root.locked && !root.lockRequested + root.strandedLock = exitCode === 0 && !root.currentlyLocked() root.recoverStrandedLock() } } @@ -833,17 +840,17 @@ Item { function lock(): string { if (!root.passwordPamConfigured) return "missing-pam" - if (!root.locked && !root.beginLock()) return "failed" + if (!root.currentlyLocked() && !root.beginLock()) return "failed" return "ok" } function isLocked(): string { - return root.locked ? "true" : "false" + return root.currentlyLocked() ? "true" : "false" } function status(): string { return JSON.stringify({ - locked: root.locked, + locked: root.currentlyLocked(), requested: root.lockRequested, pending: root.pendingSessionLock, sessionLocked: sessionLock.locked, diff --git a/test/shell.d/lock-stranded-recovery-test.sh b/test/shell.d/lock-stranded-recovery-test.sh index dec67038..d0e5cb81 100755 --- a/test/shell.d/lock-stranded-recovery-test.sh +++ b/test/shell.d/lock-stranded-recovery-test.sh @@ -32,10 +32,23 @@ assert( // omarchy-restart-shell re-locks a fresh shell, possibly mid-question. assert( - /root\.strandedLock = exitCode === 0 && !root\.locked && !root\.lockRequested/.test(serviceQml), + /root\.strandedLock = exitCode === 0 && !root\.currentlyLocked\(\)/.test(serviceQml), 'a lock this shell took while the check was in flight is not stranded' ) +// A locked binding that missed a change once read locked after an unlock, and +// the lock before suspend took that for done. Decisions read the state afresh. +assert( + /function currentlyLocked\(\) \{\s*return lockRequested \|\| sessionLock\.locked \|\| sessionLock\.secure\s*\}/.test(serviceQml), + 'the lock state is read from the session lock itself, not a cached binding' +) +assert( + /function lock\(\): string \{[\s\S]*?if \(!root\.currentlyLocked\(\) && !root\.beginLock\(\)\) return "failed"/.test(serviceQml) + && /function isLocked\(\): string \{\s*return root\.currentlyLocked\(\)/.test(serviceQml) + && /locked: root\.currentlyLocked\(\),/.test(serviceQml), + 'lock, isLocked, and status decide from the current lock state' +) + assert( /id: strandedLockRetryTimer[\s\S]*running: !root\.strandedLockResolved && remaining > 0/.test(serviceQml), 'the check retries while the compositor cannot answer, and stops once it has' @@ -60,12 +73,12 @@ assert( // A lock this shell owns ends the search. assert( - /function checkStrandedLock\(\) \{\s*if \(strandedLockResolved \|\| strandedLockCheckProc\.running\) return[\s\S]*if \(locked \|\| lockRequested\) \{\s*strandedLockResolved = true/.test(serviceQml), + /function checkStrandedLock\(\) \{\s*if \(strandedLockResolved \|\| strandedLockCheckProc\.running\) return[\s\S]*if \(currentlyLocked\(\)\) \{\s*strandedLockResolved = true/.test(serviceQml), 'a lock this shell took is not treated as stranded' ) assert( - /function recoverStrandedLock\(\) \{\s*if \(!strandedLock \|\| locked \|\| !passwordPamConfigured\) return/.test(serviceQml), + /function recoverStrandedLock\(\) \{\s*if \(!strandedLock \|\| currentlyLocked\(\) \|\| !passwordPamConfigured\) return/.test(serviceQml), 'recovery is skipped unless a stranded lock is waiting and PAM can authenticate it' )