Skip to content

fix(api): guard stdio redirect when stdin is missing (closed fd 0) - #4815

Closed
HoneyTyagii wants to merge 1 commit into
vectorize-io:mainfrom
HoneyTyagii:fix/4580-stdio-guard
Closed

HoneyTyagii wants to merge 1 commit into
vectorize-io:mainfrom
HoneyTyagii:fix/4580-stdio-guard

Conversation

@HoneyTyagii

Copy link
Copy Markdown

Summary

Root cause

Old code did:

sys.stdout.flush()
sys.stderr.flush()
with open(os.devnull, "r") as devnull:
    os.dup2(devnull.fileno(), sys.stdin.fileno())

If sys.stdin is None, the fileno() call crashes before the daemon can redirect stdio, leaving the "Failed to start daemon" 180s stall described in the issue.

Fix

  • Skip the stdin redirect when sys.stdin is None or its fileno() raises.
  • Wrap stdout and stderr flush and dup2 in the same None and OSError guards, so a closed or missing output stream cannot crash the redirect either.
  • Behavior is unchanged when all streams are valid.

Verification

  • python -m py_compile hindsight-api-slim/hindsight_api/daemon.py passes.
  • ruff check hindsight-api-slim/hindsight_api/daemon.py passes.
  • Mocked reproduction with sys.stdin=None: _redirect_stdio_to_log() completes without error and only redirects stdout and stderr (2 dup2 calls). Same for a stdin whose fileno() raises ValueError.
  • Existing hindsight-api-slim/tests/test_daemonize.py covers parent and child paths; full pytest run was not possible here because the sandbox lacks a C toolchain for optional native deps, but the touched function has no signature change.

Fixes #4580

Copilot AI lite review requested due to automatic review settings September 26, 2026 15:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@nicoloboschi

Copy link
Copy Markdown
Collaborator

Thanks for this! Closing as superseded: #4464 already fixed #4580 at the source — both spawners (hindsight_embed and hindsight_api/daemon.py) pin the child's stdin to /dev/null. The only way left to reach _redirect_stdio_to_log() with a closed fd 0 is to set the internal _HINDSIGHT_DAEMON_CHILD=1 env var by hand, which isn't a supported launch path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

3 participants