fix(sidecars): finish timeout cleanup before retrying - #1872
Conversation
|
Warning Review limit reachedNext included review available in 46 seconds. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| backend/services/subprocess_backend.py | Adds captured-child timeout handling, watchdog joining, and quarantined cleanup retries. |
| backend/tests/test_omnivoice_subprocess.py | Adds deterministic coverage for timeout reaping, replacement isolation, and cleanup quarantine behavior. |
| tests/test_resolve_heartbeat_1414.py | Uses normal backend initialization so the existing resolution-heartbeat test includes required cleanup state. |
Reviews (3): Last reviewed commit: "test: construct heartbeat backend with l..." | Re-trigger Greptile
|
|
||
| def test_timeout_never_kills_a_replacement_process(monkeypatch): | ||
| from unittest.mock import Mock | ||
| import services.subprocess_backend as module |
A sidecar timeout could return after stdout closed but before its owning process exited. A fast retry could reuse that dying process; an already-running watchdog could also target a replacement through mutable state.
The watchdog now captures its original child, kills and waits for that child with a bounded reap, and receive cleanup joins the watchdog before allowing recovery. The implementation uses the same process APIs on all supported platforms.
Validation: two deterministic regressions failed before the fix; 109 focused cross-engine tests and all 336 backend tests pass with offline, empty Hugging Face caches. Documentation updated.