Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions tools/process_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
FINISHED_TTL_SECONDS = 1800 # Keep finished processes for 30 minutes
MAX_PROCESSES = 64 # Max concurrent tracked processes (LRU pruning)
MAX_ACTIVE_PROCESS_AGE = 86400 # 24h default — see session_reset.bg_process_max_age_hours (#29177)
STALE_SWEEP_INTERVAL = 300 # Check for stale background processes every 5 minutes

# Watch pattern rate limiting — PER SESSION.
# Hard rule: at most ONE watch-match notification every WATCH_MIN_INTERVAL_SECONDS.
Expand Down Expand Up @@ -214,6 +215,17 @@ def __init__(self):
# UI view is dropped (the user can reopen it from the status stack).
self.on_close = None

# Background sweep thread — kills running processes that exceed
# MAX_ACTIVE_PROCESS_AGE to prevent leaked subprocesses from consuming
# unbounded resources (issue #76115).
self._stale_sweep_stop = threading.Event()
self._stale_sweep_thread = threading.Thread(
target=self._stale_sweep_loop,
daemon=True,
name="process-stale-sweep",
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This starts one daemon thread per ProcessRegistry instance, but _stale_sweep_loop() only exits when _stale_sweep_stop is set and this diff adds no shutdown path. The per-test ProcessRegistry fixture in tests/tools/test_process_registry.py:23-26 would accumulate retained workers; please use an existing gateway lifecycle scheduler or add explicit stop/join ownership.

self._stale_sweep_thread.start()

@staticmethod
def _clean_shell_noise(text: str) -> str:
"""Strip shell startup warnings from the beginning of output."""
Expand Down Expand Up @@ -1948,6 +1960,59 @@ def kill_all(self, task_id: str = None) -> int:
killed += 1
return killed

# ----- Stale-process sweep (issue #76115) -----

def _stale_sweep_loop(self) -> None:
"""Background daemon that kills long-running background processes.

Prevents leaked subprocesses (e.g. abandoned builds) from consuming
unbounded memory and starving the gateway event loop. Runs every
STALE_SWEEP_INTERVAL seconds and kills any running process that
exceeds MAX_ACTIVE_PROCESS_AGE.
"""
while not self._stale_sweep_stop.is_set():
try:
self._stale_sweep_stop.wait(STALE_SWEEP_INTERVAL)
if self._stale_sweep_stop.is_set():
break
self._reap_stale_running()
except Exception:
logger.debug("Stale-process sweep iteration failed", exc_info=True)

def _reap_stale_running(self, max_age: Optional[float] = None) -> int:
"""Kill running processes that exceed *max_age* seconds.

Returns the number of processes killed. Uses the existing
``kill_process`` infrastructure which handles process-tree teardown
(SIGKILL/SIGTERM on POSIX, taskkill /T /F on Windows) and moves
the session to the finished dict.
"""
if max_age is None:
max_age = float(MAX_ACTIVE_PROCESS_AGE)
now = time.time()
with self._lock:
stale_ids = [
s.id for s in self._running.values()
if not s.exited and (now - s.started_at) > max_age
]

killed = 0
for sid in stale_ids:
age_hours = None
with self._lock:
s = self._running.get(sid)
if s:
age_hours = (now - s.started_at) / 3600
logger.warning(
"Reaping stale background process %s (age %.1fh exceeds max %dh): %s",
sid, age_hours or 0, int(max_age // 3600),
s.command if s else "<unknown>",
)
result = self.kill_process(sid, source="stale_sweep", consume_output=False)
if result.get("status") in {"killed", "already_exited"}:
killed += 1
return killed

# ----- Cleanup / Pruning -----

def _prune_if_needed(self):
Expand Down
Loading