[zackees]

Refs #1429. This is the issue's "request 1": reclaim a port lock when its owning client is gone.

Mechanism

SharedSerialManager::open_port publishes the session (OS fd open, owner_client_id set) before the client attaches a reader or writer. The only automatic close is close_port_after_grace_if_idle, which runs on reader/writer detach. So when a client dies after open_port but before attaching, or the WS attach times out, the session is left like this:

[HELD] open=true owner=<id> writer=none readers=0
  client <id> pid=… (dead)

That is exactly the fbuild daemon locks output in the issue. Nothing schedules a close for it, so every other opener gets EBUSY. A later generation bump can also cancel a pending grace close without rescheduling it.

Change

• handlers::locks::reclaim_dead_client_serial_sessions closes a session only when every client it references (owner, writer, readers and the metadata entries) has a recorded pid and none of those processes is alive.
• The daemon's existing maintenance loop runs it every 2 s (DEAD_CLIENT_SERIAL_RECLAIM_INTERVAL).
• This is deliberately stricter than clear-locks --stale: a session with no readers or writer but a live or unknown-pid owner is kept, because that is the normal deploy → attach hand-off.

Verification

• New tests:
  • a session whose clients are all dead is closed
  • live-pid and unknown-pid sessions are kept
  • a dead owner with a live second reader is kept
• soldr cargo test -p fbuild-daemon (288 + 13 pass). Workspace clippy with -D warnings is clean.
• Real pty check (socat): fbuild monitor held the port, then kill -9 on the CLI, and the port was released 2.2 s later. That release came from the existing WS-disconnect path, not this sweep, which confirms the normal path is unchanged. I could not reproduce the open-then-die-before-attach window without the reporter's two-device rig, so the new path is covered by unit tests only.

Not covered

The issue's "request 2" (timeout message naming the dead client) is not in this PR; open_port_timeout_message already names the holding process.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 43 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: FastLED/fbuild
• Branch: fix/1429-reclaim-dead-client-port
• File: .coderabbit.yaml
• Commit: 2abf5e8
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5b240e2d-c32c-4386-9296-795f185dbac6

Commits

Reviewing files that changed from the base of the PR and between 6530cb6 and 2abf5e8.

Files selected for processing: 3

• crates/fbuild-daemon/src/context.rs
• crates/fbuild-daemon/src/handlers/locks.rs
• crates/fbuild-daemon/src/main.rs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}