Daemon startup is not mutually exclusive: two concurrent invocations can both bind, and the second silently orphans the first

Labels: bug, daemon, concurrency

Description

The only guard against a duplicate daemon is a check-then-act with no lock between the two halves:

# src/browser_harness/daemon.py:828-841
def already_running():
    return ipc.ping(NAME, timeout=1.0)

if __name__ == "__main__":
    if already_running():
        print(f"daemon already running on {SOCK}", file=sys.stderr)
        sys.exit(0)
    open(LOG, "w").close()
    open(PID, "w").write(str(os.getpid()))
    try:
        asyncio.run(main())

main() → Daemon.start() (CDP connect, which for a local browser can park on the Allow popup for up to LOCAL_HANDSHAKE_TIMEOUT = 45 seconds) → serve() → ipc.serve(). The window between the already_running() check and the socket actually existing is therefore seconds to tens of seconds wide.

And ipc.serve() takes the endpoint unconditionally when it gets there:

# src/browser_harness/_ipc.py:168-177
if not IS_WINDOWS:
    path = str(_sock_path(name))
    if os.path.exists(path): os.unlink(path)
    ...
    server = await asyncio.start_unix_server(handler, path=path)

os.unlink() with no ownership check: whatever was listening there is unlinked out from under itself. On Windows the equivalent is os.replace(tmp, pf) on the .port file (_ipc.py:182-185) — same outcome, the newer daemon's port/token silently replaces the older one's. pid_path is likewise overwritten by the second process.

Impact

Both ensure_daemon() (admin.py:340, called on every CLI invocation) and a direct python -m browser_harness.daemon go through this path, so any two near-simultaneous browser-harness calls sharing a BU_NAME can race:

• Two daemons end up alive against the same browser. The first is unreachable — its socket/port file is gone — so nothing will ever send it meta: shutdown.
• The orphan keeps its CDP WebSocket open, keeps its Target.attachToTarget sessions, and for a named non-cloud daemon keeps its dedicated background tab (created at daemon.py:431-436). That tab is only closed on the clean-shutdown path in serve()'s finally, which the orphan never reaches.
• For a cloud daemon the orphan holds BU_BROWSER_ID, and stop_remote() only runs on its own exit — so stop_remote_daemon(name) stops the reachable daemon and the billable browser can outlive it.
• The orphan's tap() handler keeps rewriting document.title with the marker on whatever session it still holds.

This is not a theoretical shape for this project. SKILL.md recommends parallel sub-agents, there is an open issue on parallelism (#375), and admin.ensure_daemon() spawns eagerly rather than coordinating — nothing serialises two agents that start in the same second.

Reproduction

# two shells, started together
browser-harness <<'PY'
print(page_info())
PY

or more reliably, with the handshake window widened by an un-clicked Allow popup:

for i in 1 2; do (python -m browser_harness.daemon &) ; done
sleep 5
ls ~/.config/browser-harness/runtime/       # one bu-default.sock
pgrep -fa browser_harness.daemon            # two processes

Suggested fix

Make acquisition atomic rather than checked. On POSIX the cheap version is an exclusive flock on the pid file held for the process lifetime:

lock = open(PID, "a+")
try:
    fcntl.flock(lock, fcntl.LOCK_EX | fcntl.LOCK_NB)
except OSError:
    print(f"daemon already running on {SOCK}", file=sys.stderr)
    sys.exit(0)
lock.truncate(0); lock.write(str(os.getpid())); lock.flush()

with msvcrt.locking on the Windows side. Take the lock before Daemon.start(), so the slow CDP handshake happens inside the critical section rather than in front of it.

Failing that, ipc.serve() should at minimum refuse to unlink a socket that answers ipc.ping() — the unconditional os.unlink() is what turns a duplicate-start race into a silent orphan instead of a clean "already running" exit.