fix: clean up Xvfb display after browser process crash

[dyiapanis]

The kill() method only terminated Xvfb if exitCode was null and the process was not yet killed. When the browser process crashes (SIGSEGV), exitCode is null but the process is already gone — the original check passed, but the Xvfb process was sometimes left orphaned.

Fix: always attempt kill() when the process reference exists, regardless of exit state. Wrap in try/catch for the race where the process exits between the check and the kill call. Null out the process reference after cleanup to prevent double-kill.

Discovered while running concurrent Camoufox instances with headless=virtual. After browser crashes, stale Xvfb displays accumulated on the host.

[barjin]

Thank you for your contribution to this project @dyiapanis.

kill() sets this.proc = null, but get() calls this.kill() and reads this.proc.exitCode on the next line (virtdisplay.ts:124-127), so that path throws TypeError instead of CannotExecuteXvfb. The premise also doesn't hold — a crashed child:

const p = spawn("sh", ["-c", "kill -SEGV $$"]);
p.on("exit", () => setTimeout(() =>
  console.log(p.exitCode, p.signalCode, p.killed, p.kill()), 50));
// null SIGSEGV false false

exitCode === null && !killed is already true, so the old guard passed and called kill(), and kill() returns false without throwing — the new try/catch is unreachable. The actual leak is that kill() is only reachable from the browser.close() override (utils.ts:405) while Xvfb is spawned detached: true with no exit/SIGTERM or context-disconnect handler.

Cheers!

[dyiapanis]

Thanks for the review — you're right on all counts. The original kill() changes were dead code that didn't fix the actual leak. Fixed in 293cc31:

1. get() TypeError — Reverted kill() to the original. The this.proc = null assignment caused get() to throw TypeError when reading this.proc.exitCode on the next line. Original kill() never nullified proc, so this path is safe again.

2. Unreachable try/catch — Reverted. As you noted, kill() returns false without throwing on an already-dead process, so the try/catch was unreachable. The original guard (exitCode === null && !killed) was correct.

3. The actual leak — This was the real fix. Xvfb is spawned detached: true and kill() was only reachable from the browser.close() override in syncAttachVD. When the browser process crashes (SIGSEGV), browser.close() is never called and Xvfb leaks. Fix: register disconnected and close event handlers on the browser object in syncAttachVD, so Xvfb is killed when the WebSocket drops or the process dies — not just on explicit close. Added an idempotent guard to prevent double-kill (close + disconnect can both fire).

[dyiapanis]

Hi @barjin — following up on this one as well. Your review points were addressed in 293cc31 (reverted the dead kill() changes, fixed the actual leak path in the crash cleanup handler). Let me know if the updated approach looks good or if there's anything else needed.