From 19ebcd52a177142ddda6efd7906ff5e3c486b1b6 Mon Sep 17 00:00:00 2001 From: "didericis (claude)" Date: Tue, 2 Jun 2026 10:28:24 -0400 Subject: [PATCH 1/3] docs: add PRD 0043 --- docs/prds/0043-sidecar-pipe-lifecycle.md | 74 ++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 docs/prds/0043-sidecar-pipe-lifecycle.md diff --git a/docs/prds/0043-sidecar-pipe-lifecycle.md b/docs/prds/0043-sidecar-pipe-lifecycle.md new file mode 100644 index 0000000..1e34df2 --- /dev/null +++ b/docs/prds/0043-sidecar-pipe-lifecycle.md @@ -0,0 +1,74 @@ +# PRD 0043: Sidecar Pipe Lifecycle Cleanup + +- **Status:** Draft +- **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. -- 2.52.0 From 5a2011c48ffc767d8a576f7dfee04473f4ab4e13 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 2 Jun 2026 14:53:16 +0000 Subject: [PATCH 2/3] fix: close child stdout pipes on restart and loop convergence (PRD 0043) Closes #140. In restart_daemon, the old process's stdout pipe was never explicitly closed after p.wait() returned, leaking the fd until the supervisor object was GC'd. Similarly, when the watch loop converged (all children dead), no pipe was closed. Both paths now call p.stdout.close() immediately after the process is confirmed exited. Tests enforce this with warnings.simplefilter("error", ResourceWarning) in TestSupervisor.setUp. --- bot_bottle/sidecar_init.py | 9 ++++++++- tests/unit/test_sidecar_init.py | 5 +++++ 2 files changed, 13 insertions(+), 1 deletion(-) 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/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(): -- 2.52.0 From 4fdf354b4f7b585b735c56e1114e4dfe7d22b940 Mon Sep 17 00:00:00 2001 From: claude Date: Tue, 2 Jun 2026 14:53:23 +0000 Subject: [PATCH 3/3] docs: mark PRD 0043 Active --- docs/prds/0043-sidecar-pipe-lifecycle.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/prds/0043-sidecar-pipe-lifecycle.md b/docs/prds/0043-sidecar-pipe-lifecycle.md index 1e34df2..f5b0632 100644 --- a/docs/prds/0043-sidecar-pipe-lifecycle.md +++ b/docs/prds/0043-sidecar-pipe-lifecycle.md @@ -1,6 +1,6 @@ # PRD 0043: Sidecar Pipe Lifecycle Cleanup -- **Status:** Draft +- **Status:** Active - **Author:** didericis-codex - **Created:** 2026-06-02 - **Issue:** #140 -- 2.52.0