mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-07-30 10:52:15 +00:00
fix: detach the daemon from its console on Windows
``jarvis start`` spawned the server with ``start_new_session=True``. That is
POSIX-only — CPython's Windows ``_execute_child`` names the parameter
``unused_start_new_session`` and ignores it — so on Windows the server
inherited the launching console instead of detaching from it.
Closing that console, or logging off, therefore delivered CTRL_CLOSE_EVENT
to the server. Observed in the wild as the daemon dying overnight, with
forrtl: error (200): program aborting due to window-CLOSE event
in server.log (the Fortran runtime under NumPy handles the event and
aborts). ``jarvis start`` looked like it worked: it printed a PID, wrote the
pid file and exited 0, and the server ran for as long as the console stayed
open. Registered as a log-on scheduled task, this means the machine comes
back up with no backend.
Pass DETACHED_PROCESS on Windows so the child gets no console at all, plus
CREATE_NEW_PROCESS_GROUP so a Ctrl-C in the parent console cannot reach it.
POSIX keeps start_new_session.
Verified by attaching to each spawned process with AttachConsole():
start_new_session=True attaches successfully (the child shares a console);
DETACHED_PROCESS fails with ERROR_INVALID_HANDLE (no console exists).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
08279e6b99
commit
9fc5b875d1
@@ -81,14 +81,28 @@ def start(
|
||||
if agent_name:
|
||||
cmd.extend(["--agent", agent_name])
|
||||
|
||||
# Start as background process
|
||||
# Start as background process, fully detached from the launching terminal.
|
||||
#
|
||||
# ``start_new_session`` is POSIX-only: CPython's Windows ``_execute_child``
|
||||
# names the parameter ``unused_start_new_session`` and ignores it. Relying
|
||||
# on it there leaves the server sharing its parent's console, so closing
|
||||
# that console — or logging off — delivers CTRL_CLOSE_EVENT and kills the
|
||||
# daemon. DETACHED_PROCESS gives it no console at all; the new process
|
||||
# group additionally stops a Ctrl-C in the parent reaching it.
|
||||
DEFAULT_CONFIG_DIR.mkdir(parents=True, exist_ok=True)
|
||||
log_fh = open(_LOG_FILE, "a") # noqa: SIM115
|
||||
spawn_kwargs: dict = {}
|
||||
if sys.platform == "win32":
|
||||
spawn_kwargs["creationflags"] = (
|
||||
subprocess.DETACHED_PROCESS | subprocess.CREATE_NEW_PROCESS_GROUP
|
||||
)
|
||||
else:
|
||||
spawn_kwargs["start_new_session"] = True
|
||||
proc = subprocess.Popen(
|
||||
cmd,
|
||||
stdout=log_fh,
|
||||
stderr=log_fh,
|
||||
start_new_session=True,
|
||||
**spawn_kwargs,
|
||||
)
|
||||
_write_pid(proc.pid)
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
@@ -79,3 +80,58 @@ class TestDaemonCommands:
|
||||
result = CliRunner().invoke(cli, ["start"])
|
||||
assert result.exit_code != 0
|
||||
assert "already running" in result.output
|
||||
|
||||
|
||||
class TestDaemonDetachment:
|
||||
"""The spawned server must outlive the console that started it.
|
||||
|
||||
``start_new_session`` is POSIX-only — CPython's Windows ``_execute_child``
|
||||
names the parameter ``unused_start_new_session``. Relying on it there leaves
|
||||
the server sharing its parent's console, so closing that console (or logging
|
||||
off) delivers CTRL_CLOSE_EVENT and kills the daemon.
|
||||
"""
|
||||
|
||||
@staticmethod
|
||||
def _spawn_kwargs(platform: str) -> dict:
|
||||
"""Return the kwargs ``start`` passes to Popen when spawning the server.
|
||||
|
||||
``load_config`` is stubbed because it shells out for GPU detection —
|
||||
patching Popen wholesale would otherwise break config loading before
|
||||
the spawn is reached.
|
||||
"""
|
||||
with (
|
||||
patch("openjarvis.cli.daemon_cmd._read_pid", return_value=None),
|
||||
patch("openjarvis.cli.daemon_cmd._write_pid"),
|
||||
patch("openjarvis.cli.daemon_cmd.load_config"),
|
||||
patch("openjarvis.cli.daemon_cmd.sys.platform", platform),
|
||||
patch("openjarvis.cli.daemon_cmd.subprocess.Popen") as popen,
|
||||
patch("builtins.open", MagicMock()),
|
||||
):
|
||||
popen.return_value = MagicMock(pid=4321)
|
||||
result = CliRunner().invoke(cli, ["start"])
|
||||
assert result.exit_code == 0, result.output
|
||||
spawns = [
|
||||
c for c in popen.call_args_list if c.args and "serve" in c.args[0]
|
||||
]
|
||||
assert spawns, f"start did not spawn the server: {popen.call_args_list}"
|
||||
return spawns[-1].kwargs
|
||||
|
||||
def test_windows_spawn_is_detached_from_the_console(self) -> None:
|
||||
kwargs = self._spawn_kwargs("win32")
|
||||
flags = kwargs.get("creationflags", 0)
|
||||
assert flags & subprocess.DETACHED_PROCESS, (
|
||||
"server must be spawned with DETACHED_PROCESS on Windows, otherwise "
|
||||
"closing the launching console kills it"
|
||||
)
|
||||
assert flags & subprocess.CREATE_NEW_PROCESS_GROUP, (
|
||||
"server must be in its own process group so Ctrl-C in the parent "
|
||||
"console does not propagate to it"
|
||||
)
|
||||
assert not kwargs.get("start_new_session"), (
|
||||
"start_new_session is ignored on Windows; it must not be relied on"
|
||||
)
|
||||
|
||||
def test_posix_spawn_still_uses_start_new_session(self) -> None:
|
||||
kwargs = self._spawn_kwargs("linux")
|
||||
assert kwargs.get("start_new_session") is True
|
||||
assert "creationflags" not in kwargs or kwargs["creationflags"] == 0
|
||||
|
||||
Reference in New Issue
Block a user