task-48: daemon half fixed and live; shell half shipped and measured failing
This commit is contained in:
parent
ec2f856cfb
commit
80ea42cbbb
3 changed files with 97 additions and 10 deletions
|
|
@ -1,10 +1,10 @@
|
|||
# TASK 43 — viewtop: the Souveraine compositor
|
||||
|
||||
**Status:** runs on the phone 2026-07-28. Moving under another session —
|
||||
head `a308f39f`, building the ext-session-lock admission gate. **Sync before
|
||||
touching it.** **Size:** several
|
||||
sessions, one lane each. **Repo:** `~/Projects/souveraine-viewtop`
|
||||
(`10.10.20.120:4455/Fimeg/souveraine-viewtop`), head `a308f39`.
|
||||
**Status:** runs on the phone 2026-07-28. Next lane is the ext-session-lock
|
||||
admission gate. **Size:** several sessions, one lane each. **Repo:**
|
||||
`~/Projects/souveraine-viewtop`
|
||||
(`10.10.20.120:4455/Fimeg/souveraine-viewtop`), head `0077776` = `origin/main`
|
||||
2026-07-31.
|
||||
|
||||
## Goal
|
||||
|
||||
|
|
|
|||
|
|
@ -1,8 +1,40 @@
|
|||
# TASK 48 — The shell dies on any scene reload that owes a lock
|
||||
|
||||
**Status:** open, raised 2026-07-29, fully diagnosed and reproducible.
|
||||
**Status:** half fixed, half NOT, 2026-07-31 (`efe2d0d`, `b5ee87d`).
|
||||
|
||||
- **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` (`modules/common/panels/lock/LockScreen.qml`).
|
||||
**Repo:** `souveraine` (`surfaces/quickshell/modules/common/panels/lock/LockScreen.qml`,
|
||||
`src/sessiond/server.rs`).
|
||||
|
||||
## Symptom
|
||||
|
||||
|
|
@ -42,6 +74,61 @@ 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
|
||||
|
|
@ -66,8 +153,8 @@ strictly worse — sessiond believed there was no shell for 90 minutes.
|
|||
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.* Another session is building viewtop's ext-session-lock
|
||||
admission gate — sync with it before touching this.
|
||||
to get wrong.* viewtop's ext-session-lock admission gate is the same
|
||||
mechanism — pull that repo before touching this.
|
||||
|
||||
## Acceptance
|
||||
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ Four that are cheap relative to what they unblock:
|
|||
|
||||
| # | Task | What's left |
|
||||
|---|------|-------------|
|
||||
| 43 | [viewtop: the Souveraine compositor](43-viewtop-compositor.md) | Runs on the phone. Another session is building the ext-session-lock admission gate — **sync before touching it** (head `a308f39f`). The two-client lock handoff is the thing not to get wrong. |
|
||||
| 43 | [viewtop: the Souveraine compositor](43-viewtop-compositor.md) | Runs on the phone. Next lane is the ext-session-lock admission gate. The two-client lock handoff is the thing not to get wrong. |
|
||||
| 29 | [SouveraineOS Updater](29-souveraine-updater.md) | Ships and installs by `pacman -Syu`. **Read-only on device** until something runs a polkit agent — that is the shell's job. Also owns TASK-42's layer 2. |
|
||||
|
||||
## In progress
|
||||
|
|
|
|||
Loading…
Reference in a new issue