Compare commits

...

3 Commits

Author SHA1 Message Date
didericis-claude 8ca1ede4ec docs: mark PRD 0043 Active
test / unit (pull_request) Successful in 31s
test / integration (pull_request) Successful in 44s
2026-06-02 14:53:23 +00:00
didericis-claude c2176117fa 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.
2026-06-02 14:53:16 +00:00
didericis-claude 85e73e013d docs: add PRD 0043
test / unit (pull_request) Successful in 36s
test / integration (pull_request) Successful in 58s
2026-06-02 10:28:24 -04:00
3 changed files with 87 additions and 1 deletions
+8 -1
View File
@@ -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)
+74
View File
@@ -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.
+5
View File
@@ -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():