fix(gateway): handle lifecycle edge cases
This commit is contained in:
+13
-13
@@ -178,7 +178,8 @@ class GatewayRuntime:
|
||||
self._clear_state()
|
||||
return RuntimeResult(False, "gateway_state_stale", self.status(reason="stale_state"))
|
||||
|
||||
self._terminate(status.pid, timeout_s=timeout_s)
|
||||
if not self._terminate(status.pid, timeout_s=timeout_s):
|
||||
return RuntimeResult(False, "gateway_stop_timeout", self.status(reason="stop_timeout"))
|
||||
self._clear_state()
|
||||
return RuntimeResult(True, "gateway_stopped", self.status(reason="stopped"))
|
||||
|
||||
@@ -263,13 +264,12 @@ class GatewayRuntime:
|
||||
return {"creationflags": flags}
|
||||
return {"start_new_session": True}
|
||||
|
||||
def _terminate(self, pid: int, *, timeout_s: int) -> None:
|
||||
def _terminate(self, pid: int, *, timeout_s: int) -> bool:
|
||||
if self.platform_name == "Windows":
|
||||
self._terminate_windows(pid, timeout_s=timeout_s)
|
||||
else:
|
||||
self._terminate_posix(pid, timeout_s=timeout_s)
|
||||
return self._terminate_windows(pid, timeout_s=timeout_s)
|
||||
return self._terminate_posix(pid, timeout_s=timeout_s)
|
||||
|
||||
def _terminate_posix(self, pid: int, *, timeout_s: int) -> None:
|
||||
def _terminate_posix(self, pid: int, *, timeout_s: int) -> bool:
|
||||
try:
|
||||
pgid = os.getpgid(pid)
|
||||
except OSError:
|
||||
@@ -280,28 +280,28 @@ class GatewayRuntime:
|
||||
else:
|
||||
os.kill(pid, signal.SIGTERM)
|
||||
except ProcessLookupError:
|
||||
return
|
||||
return True
|
||||
if self._wait_for_exit(pid, timeout_s):
|
||||
return
|
||||
return True
|
||||
with suppress(ProcessLookupError):
|
||||
if pgid is not None:
|
||||
os.killpg(pgid, signal.SIGKILL)
|
||||
else:
|
||||
os.kill(pid, signal.SIGKILL)
|
||||
self._wait_for_exit(pid, 2)
|
||||
return self._wait_for_exit(pid, 2)
|
||||
|
||||
def _terminate_windows(self, pid: int, *, timeout_s: int) -> None:
|
||||
def _terminate_windows(self, pid: int, *, timeout_s: int) -> bool:
|
||||
ctrl_break = getattr(signal, "CTRL_BREAK_EVENT", None)
|
||||
if ctrl_break is not None:
|
||||
with suppress(ProcessLookupError):
|
||||
os.kill(pid, ctrl_break)
|
||||
if self._wait_for_exit(pid, timeout_s):
|
||||
return
|
||||
return True
|
||||
self._subprocess_run(["taskkill", "/PID", str(pid), "/T"], check=False)
|
||||
if self._wait_for_exit(pid, 2):
|
||||
return
|
||||
return True
|
||||
self._subprocess_run(["taskkill", "/PID", str(pid), "/T", "/F"], check=False)
|
||||
self._wait_for_exit(pid, 2)
|
||||
return self._wait_for_exit(pid, 2)
|
||||
|
||||
def _wait_for_exit(self, pid: int, timeout_s: int | float) -> bool:
|
||||
deadline = time.monotonic() + max(float(timeout_s), 0.0)
|
||||
|
||||
@@ -141,7 +141,7 @@ class GatewayServiceInstaller:
|
||||
"Label": label,
|
||||
"ProgramArguments": build_gateway_command(options.python_executable, options.start),
|
||||
"WorkingDirectory": _working_directory_text(options.start),
|
||||
"RunAtLoad": bool(options.start_now),
|
||||
"RunAtLoad": bool(options.enable),
|
||||
"KeepAlive": {"SuccessfulExit": False},
|
||||
"StandardOutPath": str(stdout_path),
|
||||
"StandardErrorPath": str(stderr_path),
|
||||
@@ -149,7 +149,7 @@ class GatewayServiceInstaller:
|
||||
content = plistlib.dumps(payload, sort_keys=False).decode("utf-8")
|
||||
domain = _launchd_domain()
|
||||
commands: list[tuple[str, ...]] = []
|
||||
if options.enable or options.start_now:
|
||||
if options.start_now:
|
||||
commands.append(("launchctl", "bootstrap", domain, str(path)))
|
||||
if options.enable:
|
||||
commands.append(("launchctl", "enable", f"{domain}/{label}"))
|
||||
@@ -162,7 +162,7 @@ class GatewayServiceInstaller:
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
stdout_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text(content, encoding="utf-8")
|
||||
if options.enable or options.start_now:
|
||||
if options.start_now:
|
||||
self._run_best_effort(("launchctl", "bootout", domain, str(path)))
|
||||
for command_args in commands:
|
||||
self._subprocess_run(list(command_args), check=True)
|
||||
|
||||
@@ -103,6 +103,7 @@ def test_launchd_install_dry_run_renders_plist(tmp_path):
|
||||
"/Users/test/.nanobot/config.json",
|
||||
]
|
||||
assert payload["KeepAlive"] == {"SuccessfulExit": False}
|
||||
assert payload["RunAtLoad"] is True
|
||||
assert ("launchctl", "bootstrap", _expected_launchd_domain(), str(result.path)) in result.commands
|
||||
|
||||
|
||||
@@ -118,11 +119,34 @@ def test_launchd_no_enable_start_still_bootstraps(tmp_path):
|
||||
dry_run=True,
|
||||
)
|
||||
|
||||
assert result.content is not None
|
||||
payload = plistlib.loads(result.content.encode("utf-8"))
|
||||
assert payload["RunAtLoad"] is False
|
||||
assert result.commands[0][:2] == ("launchctl", "bootstrap")
|
||||
assert not any(command[1] == "enable" for command in result.commands)
|
||||
assert any(command[1] == "kickstart" for command in result.commands)
|
||||
|
||||
|
||||
def test_launchd_enable_without_start_sets_run_at_load_without_bootstrap(tmp_path):
|
||||
installer = GatewayServiceInstaller(platform_name="Darwin", home=tmp_path)
|
||||
|
||||
result = installer.install(
|
||||
GatewayServiceOptions(
|
||||
start=GatewayStartOptions(port=18790),
|
||||
enable=True,
|
||||
start_now=False,
|
||||
),
|
||||
dry_run=True,
|
||||
)
|
||||
|
||||
assert result.content is not None
|
||||
payload = plistlib.loads(result.content.encode("utf-8"))
|
||||
assert payload["RunAtLoad"] is True
|
||||
assert not any(command[1] == "bootstrap" for command in result.commands)
|
||||
assert any(command[1] == "enable" for command in result.commands)
|
||||
assert not any(command[1] == "kickstart" for command in result.commands)
|
||||
|
||||
|
||||
def test_launchd_no_enable_start_reinstall_boots_out_existing_label(tmp_path):
|
||||
commands: list[list[str]] = []
|
||||
installer = GatewayServiceInstaller(
|
||||
|
||||
@@ -139,10 +139,32 @@ def test_stop_terminates_recorded_process(tmp_path, monkeypatch):
|
||||
monkeypatch.setattr(runtime, "_is_pid_running", lambda _pid: True)
|
||||
monkeypatch.setattr(runtime, "_process_identity", lambda _pid: 12345)
|
||||
terminated: list[int] = []
|
||||
monkeypatch.setattr(runtime, "_terminate", lambda pid, timeout_s: terminated.append(pid))
|
||||
|
||||
def fake_terminate(pid, timeout_s):
|
||||
terminated.append(pid)
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(runtime, "_terminate", fake_terminate)
|
||||
|
||||
result = runtime.stop()
|
||||
|
||||
assert result.ok is True
|
||||
assert terminated == [12345]
|
||||
assert not runtime.paths.state_path.exists()
|
||||
|
||||
|
||||
def test_stop_keeps_state_when_process_survives_timeout(tmp_path, monkeypatch):
|
||||
runtime = GatewayRuntime(paths=_paths(tmp_path), platform_name="Linux")
|
||||
runtime.paths.run_dir.mkdir(parents=True)
|
||||
runtime.paths.state_path.write_text('{"pid": 12345, "identity": 12345}', encoding="utf-8")
|
||||
monkeypatch.setattr(runtime, "_is_pid_running", lambda _pid: True)
|
||||
monkeypatch.setattr(runtime, "_process_identity", lambda _pid: 12345)
|
||||
monkeypatch.setattr(runtime, "_terminate", lambda _pid, timeout_s: False)
|
||||
|
||||
result = runtime.stop(timeout_s=0)
|
||||
|
||||
assert result.ok is False
|
||||
assert result.message == "gateway_stop_timeout"
|
||||
assert result.status.running is True
|
||||
assert result.status.reason == "stop_timeout"
|
||||
assert runtime.paths.state_path.exists()
|
||||
|
||||
Reference in New Issue
Block a user