mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-01 12:02:05 +00:00
fix(tools): use start_new_session instead of preexec_fn to prevent SIGSEGV in multi-threaded processes
preexec_fn=os.setsid runs Python code in the forked child before exec, which is unsafe in multi-threaded processes (CPython docs). When the Desktop gateway loads native libraries (onnxruntime, BLAS, provider SDKs) with active thread pools, the fork can SIGSEGV before the child execs. Replace all preexec_fn usage with start_new_session=True, which provides the same setsid/process-group semantics without running Python in the fork. This is already the pattern used throughout hermes_cli/gateway.py and hermes_cli/_subprocess_compat.py. Fixes #46789
This commit is contained in:
parent
f0678b031e
commit
515192c4b9
5 changed files with 23 additions and 4 deletions
|
|
@ -599,7 +599,7 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
|
|||
],
|
||||
stdout=bridge_log_fh,
|
||||
stderr=bridge_log_fh,
|
||||
preexec_fn=None if _IS_WINDOWS else os.setsid,
|
||||
start_new_session=True,
|
||||
env=bridge_env,
|
||||
)
|
||||
_write_bridge_pidfile(self._session_path, self._bridge_process.pid)
|
||||
|
|
|
|||
|
|
@ -46,6 +46,25 @@ class TestNoUnconditionalSetsid:
|
|||
)
|
||||
|
||||
|
||||
class TestStartNewSession:
|
||||
"""All guarded files must use start_new_session=True instead of preexec_fn."""
|
||||
|
||||
@pytest.mark.parametrize("relpath", GUARDED_FILES)
|
||||
def test_uses_start_new_session(self, relpath):
|
||||
"""Each guarded file must use start_new_session=True for process isolation."""
|
||||
filepath = PROJECT_ROOT / relpath
|
||||
if not filepath.exists():
|
||||
pytest.skip(f"{relpath} not found")
|
||||
source = filepath.read_text(encoding="utf-8")
|
||||
# Files should use start_new_session=True, not preexec_fn
|
||||
assert "preexec_fn" not in source, (
|
||||
f"{relpath} still uses preexec_fn; use start_new_session=True instead"
|
||||
)
|
||||
assert "start_new_session=True" in source, (
|
||||
f"{relpath} missing start_new_session=True in Popen call"
|
||||
)
|
||||
|
||||
|
||||
class TestIsWindowsConstant:
|
||||
"""Each guarded file must define _IS_WINDOWS."""
|
||||
|
||||
|
|
|
|||
|
|
@ -1290,7 +1290,7 @@ def execute_code(
|
|||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.PIPE,
|
||||
stdin=subprocess.DEVNULL,
|
||||
preexec_fn=None if _IS_WINDOWS else os.setsid,
|
||||
start_new_session=True,
|
||||
creationflags=subprocess.CREATE_NO_WINDOW if _IS_WINDOWS else 0,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -731,7 +731,7 @@ class LocalEnvironment(BaseEnvironment):
|
|||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.STDOUT,
|
||||
stdin=subprocess.PIPE if stdin_data is not None else subprocess.DEVNULL,
|
||||
preexec_fn=None if _IS_WINDOWS else os.setsid,
|
||||
start_new_session=True,
|
||||
cwd=_popen_cwd,
|
||||
**_popen_kwargs,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -750,7 +750,7 @@ class ProcessRegistry:
|
|||
stdout=subprocess.PIPE,
|
||||
stderr=subprocess.STDOUT,
|
||||
stdin=subprocess.DEVNULL,
|
||||
preexec_fn=None if _IS_WINDOWS else os.setsid,
|
||||
start_new_session=True,
|
||||
**_popen_kwargs,
|
||||
)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue