diff --git a/bot_bottle/sidecar_init.py b/bot_bottle/sidecar_init.py index 8bfa984..2b303a6 100644 --- a/bot_bottle/sidecar_init.py +++ b/bot_bottle/sidecar_init.py @@ -245,7 +245,12 @@ class _Supervisor: except ProcessLookupError: pass - return all(p.poll() is not None for _, p in self.procs) + done = all(p.poll() is not None for _, p in self.procs) + if done: + for _, p in self.procs: + if p.stdout is not None: + p.stdout.close() + return done def exit_code(self) -> int: """Positive child failures win; otherwise report success. @@ -335,6 +340,8 @@ class _Supervisor: except ProcessLookupError: pass p.wait() + if p.stdout is not None: + p.stdout.close() self._logged_dead.discard(daemon_name) new_proc = _spawn(spec) self.procs[idx] = (spec, new_proc) diff --git a/docs/prds/0043-sidecar-pipe-lifecycle.md b/docs/prds/0043-sidecar-pipe-lifecycle.md new file mode 100644 index 0000000..f5b0632 --- /dev/null +++ b/docs/prds/0043-sidecar-pipe-lifecycle.md @@ -0,0 +1,74 @@ +# PRD 0043: Sidecar Pipe Lifecycle Cleanup + +- **Status:** Active +- **Author:** didericis-codex +- **Created:** 2026-06-02 +- **Issue:** #140 + +## Summary + +Close the unclosed child stdout pipe file descriptors that `sidecar_init.py` +leaks during restart and shutdown paths, eliminating `ResourceWarning` noise +and tightening the process lifecycle. + +## Problem + +Unit tests for `sidecar_init.py` pass, but restart and shutdown cases emit +`ResourceWarning: unclosed file <_io.BufferedReader …>` for child stdout pipes, +originating around lines 141 and 273. The warnings indicate the restart path +leaks pipe file descriptors: a pipe opened for a stopped or replaced child is +not explicitly closed before the next child is spawned or before the supervisor +exits. + +## Goals / Success Criteria + +- `python3 -m unittest tests.unit.test_sidecar_init` produces no + `ResourceWarning` output. +- Pipe file descriptors for stopped or replaced child processes are explicitly + closed in the restart path. +- Pipe file descriptors for all children are explicitly closed in the shutdown + path. +- No change to the external signal or exit-code contract from PRD 0034. + +## Non-goals + +- No changes to restart or shutdown policy (coalescing, ordering, timeout). +- No changes to egress, pipelock, git-gate, or supervise daemon argv. +- No new runtime dependencies. + +## Scope + +In scope: + +- `bot_bottle/sidecar_init.py` pipe open/close lifecycle in `_Supervisor`. +- Unit tests in `tests/unit/test_sidecar_init.py` asserting no leaked pipes. + +Out of scope: + +- Changing how pumping threads read from pipes. +- Integration tests that start a live sidecar container. + +## Design + +Audit every code path in `_Supervisor` where a child process is stopped, +replaced, or reaches end-of-life, and ensure the corresponding stdout pipe is +explicitly closed before spawning a replacement or exiting the supervisor loop. + +Where a pumping thread holds a reference to the pipe, coordinate closure so the +thread sees EOF and exits cleanly rather than blocking indefinitely. + +## Testing Strategy + +- Enable `ResourceWarning` as an error in test setUp: + `warnings.simplefilter("error", ResourceWarning)`. +- Run existing restart and shutdown test cases under this stricter setting. +- Add tests for restart-then-shutdown if not already covered. + +Run: + +- `python3 -m unittest tests.unit.test_sidecar_init` +- `python3 -m unittest discover -s tests/unit` + +## Open Questions + +None. diff --git a/tests/unit/test_sidecar_init.py b/tests/unit/test_sidecar_init.py index 88f0b9b..1256af0 100644 --- a/tests/unit/test_sidecar_init.py +++ b/tests/unit/test_sidecar_init.py @@ -14,6 +14,7 @@ import subprocess import sys import time import unittest +import warnings from pathlib import Path from unittest.mock import patch @@ -135,6 +136,10 @@ class TestSupervisor(unittest.TestCase): We don't go through `main()` because main installs signal handlers process-wide, which collides with the test runner.""" + def setUp(self): + warnings.simplefilter("error", ResourceWarning) + self.addCleanup(warnings.resetwarnings) + def _drive(self, sup: _Supervisor, max_wait_s: float = 6.0) -> int: deadline = time.monotonic() + max_wait_s while not sup.tick():