From 515192c4b90c934c26389da37c47877e2cd274db Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Tue, 16 Jun 2026 05:09:35 +0800 Subject: [PATCH] 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 --- plugins/platforms/whatsapp/adapter.py | 2 +- tests/tools/test_windows_compat.py | 19 +++++++++++++++++++ tools/code_execution_tool.py | 2 +- tools/environments/local.py | 2 +- tools/process_registry.py | 2 +- 5 files changed, 23 insertions(+), 4 deletions(-) diff --git a/plugins/platforms/whatsapp/adapter.py b/plugins/platforms/whatsapp/adapter.py index 05daeabeed8..cc5f5f95ac2 100644 --- a/plugins/platforms/whatsapp/adapter.py +++ b/plugins/platforms/whatsapp/adapter.py @@ -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) diff --git a/tests/tools/test_windows_compat.py b/tests/tools/test_windows_compat.py index ec04d209593..5c7e920aae3 100644 --- a/tests/tools/test_windows_compat.py +++ b/tests/tools/test_windows_compat.py @@ -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.""" diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index 5749b224bdf..6fc95c25434 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -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, ) diff --git a/tools/environments/local.py b/tools/environments/local.py index 0a1651a758b..9a0558e5af1 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -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, ) diff --git a/tools/process_registry.py b/tools/process_registry.py index 69bfc13619d..e21c68af993 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -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, )