Skip to content

fix(embed): pin stdin to DEVNULL when spawning the daemon - #4464

Merged
nicoloboschi merged 2 commits into
vectorize-io:mainfrom
E-R-Butch:fix/embed-daemon-spawn-stdin
Sep 25, 2026
Merged

nicoloboschi merged 2 commits into
vectorize-io:mainfrom
E-R-Butch:fix/embed-daemon-spawn-stdin

Conversation

@E-R-Butch

Copy link
Copy Markdown
Contributor

_detach_popen_kwargs pins stdin on Windows but not on POSIX, so the daemon child inherits the caller's fd 0. When that fd 0 is a socket opened with FD_CLOEXEC (e.g. a TUI/gateway parent that wires its IPC channel onto fds 0-2), the kernel closes it at exec: the child starts with sys.stdin = None and dies in _redirect_stdio_to_log() with

AttributeError: 'NoneType' object has no attribute 'fileno'

The manager then waits out its startup timeout and reports "Daemon failed to start". The sibling helper in hindsight-api-slim/hindsight_api/daemon.py already pins stdin; only this copy had drifted.

Repro (no daemon involved):

import fcntl, os, socket, subprocess, sys
s1, s2 = socket.socketpair()
pid = os.fork()
if pid == 0:
    os.dup2(s1.fileno(), 0)
    fcntl.fcntl(0, fcntl.F_SETFD, fcntl.FD_CLOEXEC)
    subprocess.Popen([sys.executable, "-c", "import sys; print(sys.stdin)"], start_new_session=True)
    os._exit(0)
os.waitpid(pid, 0)  # prints None for the child on current main

Pins stdin on both platforms and adds a regression test (test_detach_popen_kwargs_pins_stdin, fails on main, passes with the fix). Verified on macOS / Python 3.11: a daemon start from a CLOEXEC-fd0 parent comes up healthy.

E-R-Butch and others added 2 commits September 25, 2026 14:06
The POSIX branch of `_detach_popen_kwargs` left `stdin` unset, so the
daemon child inherited the caller's fd 0. A caller can hold an fd 0 that
is a socket opened with FD_CLOEXEC (e.g. a TUI/gateway that wires its IPC
channel onto fds 0-2); the kernel closes an inherited fd 0 at exec, so the
child starts with `sys.stdin = None` and dies in `_redirect_stdio_to_log()`
with `AttributeError: 'NoneType' object has no attribute 'fileno'`.

The Windows branch already pinned stdin, and so does the sibling helper in
hindsight-api-slim/hindsight_api/daemon.py — the two copies had drifted.
Review fix: assert kwargs["stdin"] instead of kwargs.get("stdin"), so a
missing key fails loudly as a KeyError.
@nicoloboschi
nicoloboschi force-pushed the fix/embed-daemon-spawn-stdin branch from e9233ad to 13e165d Compare September 25, 2026 12:20
@nicoloboschi

Copy link
Copy Markdown
Collaborator

Thanks! Rebased onto main and pushed one small review fix: the regression test now indexes kwargs["stdin"] directly so a missing key fails loudly. Full CI (including embed on Linux/Windows) is green apart from the unrelated hermes-compat job (upstream hermes-agent now needs ruamel).

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