lock: adopt the session lock across a scene reload
sessiond admits shell_ready from the lease holder's own pid; the shell carries its lock request through the reload so WlSessionLock adopts the existing lock instead of taking the unlock branch on it.
This commit is contained in:
parent
0745b48dc5
commit
efe2d0d1e0
2 changed files with 211 additions and 1 deletions
|
|
@ -54,6 +54,12 @@ struct Daemon {
|
|||
/// its generation is still current (a restarted shell supersedes).
|
||||
heartbeat_gen: u64,
|
||||
shell_alive: bool,
|
||||
/// SO_PEERCRED pid of the process holding the authority lease. The lease
|
||||
/// is per-connection, but the *shell* is a process, and a quickshell scene
|
||||
/// reload opens a second connection from the same pid while the first is
|
||||
/// still open. Without this the daemon cannot tell its own shell's
|
||||
/// successor from an impostor, and refuses the successor forever (TASK-48).
|
||||
shell_pid: Option<i32>,
|
||||
/// Write handle to the live shell's heartbeat connection, so the authority
|
||||
/// can push a directive down it instead of only answering requests. The
|
||||
/// shell owns the rich lock surface in steady state; this is how the
|
||||
|
|
@ -109,6 +115,7 @@ pub fn run(initial_lock: bool, socket_path: &Path) -> Result<()> {
|
|||
controller: None,
|
||||
heartbeat_gen: 0,
|
||||
shell_alive: false,
|
||||
shell_pid: None,
|
||||
shell_directives: None,
|
||||
device_state: DeviceStateMachine::new(),
|
||||
}),
|
||||
|
|
@ -602,6 +609,7 @@ fn handle_connection(stream: UnixStream, shared: Arc<Shared>) {
|
|||
let mut d = shared.lock();
|
||||
if d.heartbeat_gen == gen && d.shell_alive {
|
||||
d.shell_alive = false;
|
||||
d.shell_pid = None;
|
||||
d.shell_directives = None;
|
||||
let holding = d.controller.is_some();
|
||||
drop(d);
|
||||
|
|
@ -613,6 +621,13 @@ fn handle_connection(stream: UnixStream, shared: Arc<Shared>) {
|
|||
}
|
||||
}
|
||||
|
||||
/// SO_PEERCRED pid of the process on the other end, or None if the kernel
|
||||
/// would not say. None is never treated as a match: the lease is exclusive and
|
||||
/// "I could not prove who you are" has to fail closed.
|
||||
fn peer_pid(stream: &UnixStream) -> Option<i32> {
|
||||
stream.peer_cred().ok().and_then(|c| c.pid)
|
||||
}
|
||||
|
||||
fn handle_request(
|
||||
req: Request,
|
||||
shared: &Arc<Shared>,
|
||||
|
|
@ -641,6 +656,7 @@ fn handle_request(
|
|||
})
|
||||
}
|
||||
Request::ShellReady => {
|
||||
let peer = directives.and_then(peer_pid);
|
||||
let mut d = shared.lock();
|
||||
|
||||
// Session authority is an exclusive lease. A utility QML process
|
||||
|
|
@ -649,11 +665,56 @@ fn handle_request(
|
|||
// process's EOF into a false shell-death lock. The current
|
||||
// authority must disconnect first; its connection handler clears
|
||||
// shell_alive before a replacement can register.
|
||||
//
|
||||
// One exception, and it is half of TASK-48: a quickshell scene
|
||||
// reload is the SAME process. The QML tree is rebuilt and opens a
|
||||
// second connection while the outgoing tree's socket is still open
|
||||
// — same pid, unit still `active`, NRestarts still 0 — so the old
|
||||
// connection's EOF is minutes away or never comes at all. Refusing
|
||||
// that registration refuses the shell's own successor, and the
|
||||
// measured result is a lease that never frees: "shell_ready
|
||||
// refused ... retrying (24)" every 5s forever, black screen, no
|
||||
// crash to notice. Only a service restart clears it.
|
||||
//
|
||||
// SO_PEERCRED is the kernel's answer to who is on the other end,
|
||||
// and it is the one thing in this exchange that cannot lie — the
|
||||
// same move lockhint.rs makes by reading logind instead of
|
||||
// mirroring it. Same pid as the holder means reload; supersede.
|
||||
// Anything we cannot PROVE is the same process is still refused,
|
||||
// so the lease keeps every case it was written for.
|
||||
if d.shell_alive {
|
||||
return refuse(RefusalCode::RefusedByState, "shell authority is already registered");
|
||||
let is_reload = matches!((peer, d.shell_pid), (Some(new), Some(held)) if new == held);
|
||||
if !is_reload {
|
||||
return refuse(RefusalCode::RefusedByState, "shell authority is already registered");
|
||||
}
|
||||
// Supersede in place. `heartbeat_gen` already exists to make a
|
||||
// stale connection's EOF harmless — the outgoing handler checks
|
||||
// `heartbeat_gen == gen` before declaring the shell dead — so
|
||||
// bumping it here is exactly what stops the old socket, closing
|
||||
// whenever the old scene is finally collected, from locking a
|
||||
// session the new tree is already holding.
|
||||
d.heartbeat_gen += 1;
|
||||
d.shell_directives = directives.and_then(|s| s.try_clone().ok());
|
||||
let gen = d.heartbeat_gen;
|
||||
*heartbeat = Some(gen);
|
||||
// The daemon is not holding the lock across a reload — the
|
||||
// shell is, and misc:allow_session_lock_restore is what lets
|
||||
// the new tree adopt it. So `held` is false and there is no
|
||||
// handoff to wait on. `must_lock` still reports the session's
|
||||
// real state, because a tree that came up without a lock
|
||||
// request has to raise one and this is the answer it acts on.
|
||||
let locked = crate::sessiond::device_state::is_locked(d.device_state.state());
|
||||
info!(
|
||||
"shell re-registered after scene reload (pid {}, gen {gen}, session locked={locked})",
|
||||
peer.unwrap_or(-1)
|
||||
);
|
||||
return serde_json::json!({
|
||||
"ok": true, "held": false, "reload": true, "must_lock": locked
|
||||
});
|
||||
}
|
||||
d.heartbeat_gen += 1;
|
||||
d.shell_alive = true;
|
||||
d.shell_pid = peer;
|
||||
d.shell_directives = directives.and_then(|s| s.try_clone().ok());
|
||||
let gen = d.heartbeat_gen;
|
||||
*heartbeat = Some(gen);
|
||||
|
|
@ -952,6 +1013,7 @@ mod tests {
|
|||
controller: None,
|
||||
heartbeat_gen: 0,
|
||||
shell_alive: false,
|
||||
shell_pid: None,
|
||||
shell_directives: None,
|
||||
device_state: DeviceStateMachine::new(),
|
||||
}),
|
||||
|
|
@ -978,4 +1040,83 @@ mod tests {
|
|||
assert_eq!(state.heartbeat_gen, 1);
|
||||
assert_eq!(state.phase, Phase::Released);
|
||||
}
|
||||
|
||||
/// TASK-48. A scene reload is the same pid on a second connection while
|
||||
/// the first is still open. It must be admitted, or the lease never frees
|
||||
/// and the phone sits black until the unit is restarted.
|
||||
#[test]
|
||||
fn scene_reload_from_the_same_pid_supersedes_the_lease() {
|
||||
let shared = idle_shared();
|
||||
let (outgoing, _o) = UnixStream::pair().unwrap();
|
||||
let (incoming, _i) = UnixStream::pair().unwrap();
|
||||
|
||||
let mut first_heartbeat = None;
|
||||
let first = handle_request(
|
||||
Request::ShellReady,
|
||||
&shared,
|
||||
Some(&outgoing),
|
||||
&mut first_heartbeat,
|
||||
);
|
||||
assert_eq!(first["ok"], true);
|
||||
assert_eq!(first_heartbeat, Some(1));
|
||||
assert_eq!(shared.lock().shell_pid, Some(std::process::id() as i32));
|
||||
|
||||
// The outgoing connection has NOT closed — this is the whole point.
|
||||
let mut reload_heartbeat = None;
|
||||
let reload = handle_request(
|
||||
Request::ShellReady,
|
||||
&shared,
|
||||
Some(&incoming),
|
||||
&mut reload_heartbeat,
|
||||
);
|
||||
assert_eq!(reload["ok"], true, "same-pid reload must not be refused");
|
||||
assert_eq!(reload["reload"], true);
|
||||
assert_eq!(reload_heartbeat, Some(2));
|
||||
|
||||
let state = shared.lock();
|
||||
assert!(state.shell_alive, "the shell never stopped being alive");
|
||||
assert_eq!(state.heartbeat_gen, 2);
|
||||
}
|
||||
|
||||
/// The generation bump is what disarms the outgoing connection: when the
|
||||
/// old scene is finally collected and its socket EOFs, that handler must
|
||||
/// not declare the shell dead and lock a session the new tree is holding.
|
||||
#[test]
|
||||
fn stale_connection_eof_after_a_reload_does_not_kill_the_shell() {
|
||||
let shared = idle_shared();
|
||||
let (outgoing, _o) = UnixStream::pair().unwrap();
|
||||
let (incoming, _i) = UnixStream::pair().unwrap();
|
||||
|
||||
let mut stale_heartbeat = None;
|
||||
handle_request(Request::ShellReady, &shared, Some(&outgoing), &mut stale_heartbeat);
|
||||
let mut live_heartbeat = None;
|
||||
handle_request(Request::ShellReady, &shared, Some(&incoming), &mut live_heartbeat);
|
||||
|
||||
// Replays the EOF branch of handle_connection for the OLD connection.
|
||||
let stale_gen = stale_heartbeat.unwrap();
|
||||
let mut d = shared.lock();
|
||||
let would_lock = d.heartbeat_gen == stale_gen && d.shell_alive;
|
||||
assert!(!would_lock, "a superseded connection's EOF must be inert");
|
||||
assert!(d.shell_alive);
|
||||
assert_eq!(live_heartbeat, Some(d.heartbeat_gen));
|
||||
d.shell_pid = None; // silence the unused-mut lint path
|
||||
}
|
||||
|
||||
/// A caller we cannot identify never inherits the reload exemption, even
|
||||
/// while a real shell holds the lease. Unprovable identity fails closed.
|
||||
#[test]
|
||||
fn unidentified_caller_is_still_refused_while_a_shell_holds_the_lease() {
|
||||
let shared = idle_shared();
|
||||
let (shell, _s) = UnixStream::pair().unwrap();
|
||||
|
||||
let mut shell_heartbeat = None;
|
||||
handle_request(Request::ShellReady, &shared, Some(&shell), &mut shell_heartbeat);
|
||||
|
||||
let mut anon_heartbeat = None;
|
||||
let anon = handle_request(Request::ShellReady, &shared, None, &mut anon_heartbeat);
|
||||
assert_eq!(anon["ok"], false);
|
||||
assert_eq!(anon["reason"], "shell authority is already registered");
|
||||
assert_eq!(anon_heartbeat, None);
|
||||
assert_eq!(shared.lock().heartbeat_gen, 1);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -66,6 +66,10 @@ Scope {
|
|||
target: GlobalStates
|
||||
function onScreenLockedChanged() {
|
||||
Persistent.states.lock.locked = GlobalStates.screenLocked;
|
||||
// Persistent is a file and answers asynchronously, so it can
|
||||
// never be the thing a reload reads at construction. This is
|
||||
// the same fact held in-process, where the reload can see it.
|
||||
lockContinuity.held = GlobalStates.screenLocked;
|
||||
if (GlobalStates.screenLocked) {
|
||||
lockContext.reset();
|
||||
lockContext.tryFingerUnlock();
|
||||
|
|
@ -101,8 +105,54 @@ Scope {
|
|||
}
|
||||
}
|
||||
|
||||
// TASK-48. The lock REQUEST, carried across a quickshell scene reload by
|
||||
// quickshell's own reload machinery — in-process and synchronous, which is
|
||||
// what this has to be. Declared before the WlSessionLock on purpose:
|
||||
// `Scope` is a ReloadPropagator and reloads its children in declaration
|
||||
// order, so `held` is restored and applied before the lock below reloads.
|
||||
//
|
||||
// Why any of this is needed, from reading quickshell's session_lock.cpp:
|
||||
// on reload the new WlSessionLock adopts the outgoing one's
|
||||
// SessionLockManager and 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. A scene reload therefore
|
||||
// UNLOCKS the session. What is left is a compositor with no lock surface,
|
||||
// a phone that reads black, and `{"locked":false,"lockRequested":true}`.
|
||||
// If instead the adoption does not match, the new manager's lock() is
|
||||
// refused (the outgoing lock is still the process-global holder) and
|
||||
// updateSurfaces(true) is called anyway — that is the FATAL this task was
|
||||
// raised for. Same root cause, two faces.
|
||||
//
|
||||
// With the request true at construction, realizeLockTarget takes the adopt
|
||||
// branch: surfaces are re-created 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 task's "adopting beats re-requesting",
|
||||
// and it is the only branch that never opens the panel.
|
||||
PersistentProperties {
|
||||
id: lockContinuity
|
||||
reloadableId: "souveraineLockContinuity"
|
||||
|
||||
property bool held: false
|
||||
|
||||
onLoaded: {
|
||||
if (lockContinuity.held && !GlobalStates.screenLocked) {
|
||||
console.log("[lock] scene reload owed a lock — adopting, not re-requesting");
|
||||
GlobalStates.screenLocked = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
WlSessionLock {
|
||||
id: lock
|
||||
// Explicit so the adoption survives being reparented out of a Scope.
|
||||
// Under a Scope children match by index and this is unused; anywhere
|
||||
// else an empty reloadableId means oldInstance is ALWAYS null, the
|
||||
// manager is never adopted, and the lock cannot be re-acquired for the
|
||||
// life of the process.
|
||||
reloadableId: "souveraineSessionLock"
|
||||
locked: GlobalStates.screenLocked
|
||||
surface: root.sessionLockSurface
|
||||
|
||||
|
|
@ -155,6 +205,25 @@ Scope {
|
|||
}
|
||||
}
|
||||
|
||||
// `secure` is an EDGE, and after a reload that adopts the lock that edge
|
||||
// is in the past: the compositor acked before this tree existed, so
|
||||
// onSecureChanged never fires and GlobalStates.screenLockSecure would sit
|
||||
// false on a session that is genuinely secure. Everything gating on it —
|
||||
// redaction, capability tiers, and the locked_ack this handoff owes
|
||||
// sessiond — would then be wrong in the dangerous direction.
|
||||
//
|
||||
// Publish the LEVEL once the reload has actually run. Component.onCompleted
|
||||
// is too early: Reloadable defers onReload to after component completion,
|
||||
// so `lock.secure` is still false there. Setting screenLockSecure is enough
|
||||
// to send the ack — SessiondBridge is already listening for it.
|
||||
Connections {
|
||||
target: Quickshell
|
||||
function onReloadCompleted() {
|
||||
GlobalStates.screenLockSecure = lock.secure;
|
||||
root.dismissBloomIfSecure();
|
||||
}
|
||||
}
|
||||
|
||||
// Idempotent, and deliberately NOT a timeout. A bloom that outlives its
|
||||
// reason is a bug to locate, not something to paper over with a timer — if
|
||||
// this is still up while the lock is secure, the caller is missing and the
|
||||
|
|
|
|||
Loading…
Reference in a new issue