Skip to content

fix(daemon): restore instance-lock recovery from a wedged daemon - #165

Closed
neumie wants to merge 1 commit into
refactorx/full-headlessfrom
fix/daemon-lock-reap-verification
Closed

fix(daemon): restore instance-lock recovery from a wedged daemon#165
neumie wants to merge 1 commit into
refactorx/full-headlessfrom
fix/daemon-lock-reap-verification

Conversation

@neumie

@neumie neumie commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Problem

Starting Okena while a UI-owned daemon held the instance lock without advertising ended in a hard lockout — the user had to kill the process by hand:

INFO  Instance lock held but no daemon advertised; waiting for the owner to re-advertise or exit before spawning
WARN  Instance lock owner pid 95630 neither re-advertised nor exited within 8s; reaping it
WARN  Refusing to reap pid 95630: process is not a verified UI-owned Okena daemon
Failed to start local daemon: Instance lock is held by pid 95630, but it is not a verified UI-owned Okena daemon; refusing to terminate it.

The process genuinely was Okena.

Root cause 1 — the guard could never succeed

kill_pid_if_ui_owned_okena verified ownership by looking for --ui-owned in the owner's argv, read via sysinfo:

sys.refresh_processes(ProcessesToUpdate::Some(&[spid]), true);
// ...
let ui_owned = proc.cmd().iter().any(|arg| arg.to_str() == Some("--ui-owned"));

System::refresh_processes refreshes only a minimal set of fields and leaves cmd() emptyname and exe are populated, argv is not. Verified with a probe against sysinfo 0.33 on macOS:

refresh_processes (current): name="sleep" cmd=[]
with_cmd(Always) (fixed):    name="sleep" cmd=["/bin/sleep", "30"]

So ui_owned was always false, the function could never return true, and the entire reap path was unreachable. Every un-advertised lock holder became a lockout. The command line has to be requested explicitly.

Root cause 2 — exposed by fixing the first

Fixing that made the post-reap check reachable for the first time, and it was wrong:

std::thread::sleep(Duration::from_millis(200));
if instance_lock_is_held() { return Err(...); }

It failed on "is the lock held by anyone", not "is it still held by the process we just killed". The likeliest reason a daemon wedges un-advertised is a restart in flight — and in that state its replacement is already running, blocked in wait_for_pid_exit(old_pid, 10s), waiting for exactly this kill to acquire the lock. It takes over well inside the 200ms window, so a successful recovery reported itself as the same lockout, now naming a pid that no longer existed.

Now: wait for the reaped pid to actually exit (that is what releases the flock) instead of sleeping a fixed guess; fail only if that pid still holds the lock; and give a successor its own settle window. Bounded to 2 reaps so a pathological chain of wedged daemons still terminates rather than reaping forever.

Structure

Process inspection moves behind read_process_identity + a pure is_ui_owned_okena predicate, so both are testable. UI_OWNED_FLAG replaces the bare string literal at the spawn site and the check.

Tests

  • Predicate table: okena + flag → reap; okena without the flag (a daemon the user started themselves) → refuse; recycled pid carrying the flag but not okena → refuse; unknown process → refuse.
  • A unix test spawns a child and asserts its argv is readable. This fails on the pre-fix code with args: [] — I verified by temporarily reverting the call.

Full build clean, clippy clean workspace-wide, 1367 tests pass.

Notes

Stacked on #157 — base is refactorx/full-headless.

Follow-up, tracked separately: is_process_alive is a bare kill(pid, 0), which reports zombies as alive. The GUI holds its daemon's Child without ever reaping it, so on every clean quit wait_for_pid_exit burns its full 3s on a corpse and then logs a false "did not exit gracefully"; it also makes a crashed daemon look live, sending startup down the attach branch into a hard error instead of respawning.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y4Aq3X2s6ftafXzmqaN3A8

Starting Okena while a UI-owned daemon held the instance lock without
advertising ended in a hard lockout the user had to clear by killing the
process by hand:

  WARN  Instance lock owner pid 95630 neither re-advertised nor exited
        within 8s; reaping it
  WARN  Refusing to reap pid 95630: process is not a verified UI-owned
        Okena daemon

The guard verified ownership by looking for `--ui-owned` in the owner's
argv, read via sysinfo after `System::refresh_processes`. That call
refreshes only a minimal set of fields and leaves `cmd()` empty — `name`
and `exe` are populated, argv is not. The flag was therefore never found,
no process ever verified, and the whole reap path was unreachable. Ask
for the command line explicitly.

That made the post-reap check reachable for the first time, and it was
wrong: it failed on "is the lock held by anyone", not "is it still held
by the process we just killed". The likeliest reason a daemon wedges
un-advertised is a restart in flight — and in that state its replacement
is already running, blocked on `--await-pid <old>`, waiting for exactly
this kill to acquire the lock. So a successful recovery reported itself
as the same lockout, now naming a pid that no longer existed. Wait for
the reaped pid to actually exit, fail only if that pid still holds the
lock, and give a successor its own window to advertise.

Process inspection moves behind `read_process_identity` + a pure
`is_ui_owned_okena` predicate so both are testable; the unix test spawns
a child and asserts its argv is readable, which fails on the old call
with `args: []`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4Aq3X2s6ftafXzmqaN3A8
@matej21

matej21 commented Jul 21, 2026

Copy link
Copy Markdown
Member

Integrated into #157 as commit e4358d6. Review follow-up e3034c8 hardens the lock handoff by avoiding stale lockfile-owner inference and refusing to spawn against a still-held lock.

@matej21 matej21 closed this Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants