mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(pty-bridge): terminate PTY process groups on teardown
This commit is contained in:
parent
e9c1e757fe
commit
b31c6c33b2
2 changed files with 82 additions and 2 deletions
|
|
@ -250,13 +250,23 @@ class PtyBridge:
|
|||
return
|
||||
self._closed = True
|
||||
|
||||
try:
|
||||
pgid = os.getpgid(self._proc.pid)
|
||||
except Exception:
|
||||
pgid = None
|
||||
|
||||
# SIGHUP is the conventional "your terminal went away" signal.
|
||||
# We escalate if the child ignores it.
|
||||
# Send it to the whole foreground process group, not just the PTY
|
||||
# leader: the dashboard TUI starts helper children such as the Python
|
||||
# slash worker, and killing only the leader can strand those helpers.
|
||||
for sig in (signal.SIGHUP, signal.SIGTERM, signal.SIGKILL): # windows-footgun: ok — POSIX-only module (imports fcntl/termios/ptyprocess at top)
|
||||
if not self._proc.isalive():
|
||||
break
|
||||
try:
|
||||
self._proc.kill(sig)
|
||||
if pgid is not None:
|
||||
os.killpg(pgid, sig)
|
||||
else:
|
||||
self._proc.kill(sig)
|
||||
except Exception:
|
||||
pass
|
||||
deadline = time.monotonic() + 0.5
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ from __future__ import annotations
|
|||
|
||||
import os
|
||||
import shutil
|
||||
import signal
|
||||
import sys
|
||||
import time
|
||||
|
||||
|
|
@ -211,6 +212,75 @@ class TestPtyBridgeClose:
|
|||
break
|
||||
assert reaped, f"pid {pid} still running after close()"
|
||||
|
||||
def test_close_signals_child_process_group(self, monkeypatch):
|
||||
sent: list[tuple[int, signal.Signals]] = []
|
||||
|
||||
class _FakeProc:
|
||||
pid = 12345
|
||||
fd = -1
|
||||
|
||||
def __init__(self):
|
||||
self.alive = True
|
||||
|
||||
def isalive(self):
|
||||
return self.alive
|
||||
|
||||
def kill(self, sig):
|
||||
raise AssertionError(f"single-process kill used: {sig}")
|
||||
|
||||
def close(self, force=False):
|
||||
self.closed = force
|
||||
|
||||
fake = _FakeProc()
|
||||
|
||||
def fake_killpg(pgid, sig):
|
||||
sent.append((pgid, sig))
|
||||
fake.alive = False
|
||||
|
||||
monkeypatch.setattr(os, "getpgid", lambda pid: 67890)
|
||||
monkeypatch.setattr(os, "killpg", fake_killpg)
|
||||
|
||||
bridge = PtyBridge.__new__(PtyBridge)
|
||||
bridge._proc = fake
|
||||
bridge._fd = -1
|
||||
bridge._closed = False
|
||||
|
||||
bridge.close()
|
||||
|
||||
assert sent == [(67890, signal.SIGHUP)]
|
||||
assert bridge._closed is True
|
||||
|
||||
def test_close_falls_back_to_single_process_signal_when_group_unknown(self, monkeypatch):
|
||||
sent: list[signal.Signals] = []
|
||||
|
||||
class _FakeProc:
|
||||
pid = 12345
|
||||
fd = -1
|
||||
|
||||
def __init__(self):
|
||||
self.alive = True
|
||||
|
||||
def isalive(self):
|
||||
return self.alive
|
||||
|
||||
def kill(self, sig):
|
||||
sent.append(sig)
|
||||
self.alive = False
|
||||
|
||||
def close(self, force=False):
|
||||
self.closed = force
|
||||
|
||||
monkeypatch.setattr(os, "getpgid", lambda pid: (_ for _ in ()).throw(OSError()))
|
||||
|
||||
bridge = PtyBridge.__new__(PtyBridge)
|
||||
bridge._proc = _FakeProc()
|
||||
bridge._fd = -1
|
||||
bridge._closed = False
|
||||
|
||||
bridge.close()
|
||||
|
||||
assert sent == [signal.SIGHUP]
|
||||
|
||||
|
||||
@skip_on_windows
|
||||
class TestPtyBridgeEnv:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue