Skip to content

Commit 7241849

Browse files
committed
fix(process-runtime): keep a short cleanup budget short
retire_owned_process floored the native-termination share at a 50 ms grace, so a caller asking for 20 ms received more time than its own deadline. Derive the inner budget from what is left of the caller's deadline instead, and cover the sub-grace case so the shared entry point keeps that promise. Signed-off-by: ChengLong GU <88359731+gcl-coder@users.noreply.github.com>
1 parent efe5b5f commit 7241849

2 files changed

Lines changed: 35 additions & 3 deletions

File tree

‎loopx/extensions/process_runtime.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -402,13 +402,16 @@ def retire_owned_process(
402402
if not math.isfinite(timeout) or timeout <= 0:
403403
raise ValueError("native process cleanup timeout must be positive and finite")
404404
deadline = time.monotonic() + timeout
405+
# Split one budget: native termination gets most of it, the remainder is
406+
# left for pipe handles a Windows reader thread may still own. The inner
407+
# budget is derived from what is left of the caller's deadline and never
408+
# widened to a floor, so a sub-50 ms cleanup stays sub-50 ms.
409+
terminate_timeout = min(timeout * 0.8, deadline - time.monotonic())
405410
try:
406-
# Reserve part of the single cleanup budget for the pipe handles that a
407-
# Windows reader thread may still own.
408411
force_terminate_process(
409412
process,
410413
process_tree_owned=process_tree_owned,
411-
timeout=max(0.05, timeout * 0.8),
414+
timeout=max(terminate_timeout, 1e-6),
412415
)
413416
finally:
414417
_close_owned_streams_before_deadline(process, deadline)

‎tests/extensions/test_process_runtime.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import loopx.extensions.process_runtime as process_runtime
1818
from loopx.extensions.process_runtime import (
1919
force_terminate_process,
20+
retire_owned_process,
2021
remove_owned_tree,
2122
run_capped_process,
2223
terminate_process_tree,
@@ -224,6 +225,34 @@ def collect(*, timeout):
224225
observer.stdout.close.assert_called_once_with()
225226

226227

228+
class _NoStreamsProcess:
229+
"""Stand-in for a Popen whose pipes ``communicate`` already released."""
230+
231+
stdin = None
232+
stdout = None
233+
stderr = None
234+
235+
236+
def test_retire_owned_process_never_widens_a_sub_grace_budget(monkeypatch):
237+
"""A 20 ms cleanup stays inside 20 ms; it is not raised to a 50 ms floor."""
238+
239+
process = _NoStreamsProcess()
240+
captured = {}
241+
242+
def record_termination(process, *, process_tree_owned, timeout):
243+
captured["timeout"] = timeout
244+
captured["owned"] = process_tree_owned
245+
246+
monkeypatch.setattr(process_runtime, "force_terminate_process", record_termination)
247+
started = time.monotonic()
248+
retire_owned_process(process, process_tree_owned=True, timeout=.02)
249+
elapsed = time.monotonic() - started
250+
251+
assert captured["owned"] is True
252+
assert 0 < captured["timeout"] <= .016
253+
assert elapsed < .05
254+
255+
227256
@pytest.mark.parametrize("timeout", [0, -1, float("nan"), float("inf")])
228257
def test_force_cleanup_rejects_an_unbounded_or_invalid_budget(timeout):
229258
process = Mock(spec=subprocess.Popen)

0 commit comments

Comments
 (0)