fix(bot-screen): a start() that times out takes its launcher down instead of leaving it to publish later
_spawn_and_wait raised on the readiness deadline and walked away from the launcher it had just spawned. Xvnc and Xfce kept coming up; a moment later the launcher published DISPLAY and rfb.sock, and a screen whose start() had reported failure was now "running" under a pid the caller never learned about. A retry then saw it as already up and returned it, so the failure message described a state that no longer existed. On timeout the launch's process group (the launcher is its own session leader, so the group is exactly this launch) gets SIGTERM, then SIGKILL after a grace period, the child is reaped, and only that launch's launcher.pid / env / rfb.sock are removed before the error propagates.
This commit is contained in:
@@ -221,3 +221,36 @@ def test_orphaned_x_server_of_a_dead_launcher_is_reaped_on_next_start(in_process
|
||||
assert second.pid != first.pid and second.running
|
||||
assert _wait_until(lambda: _gone(orphan)), "the dead launcher's X server must be reaped, not leaked"
|
||||
assert runtime.stop() is True
|
||||
|
||||
|
||||
_SLOW_LAUNCHER = """#!/usr/bin/env bash
|
||||
# Publishes only AFTER runtime.start()'s readiness deadline has passed.
|
||||
sleep 30 &
|
||||
echo $! > "$HERMES_BD_XLOCK_DIR/.X${HERMES_BD_DISPLAY_NUM}-lock"
|
||||
sleep 1
|
||||
: > "$HERMES_BD_SOCKET"
|
||||
printf 'DISPLAY=:%s\\n' "$HERMES_BD_DISPLAY_NUM" > "$HERMES_BD_ENV_FILE"
|
||||
wait
|
||||
"""
|
||||
|
||||
|
||||
@pytest.mark.linux_only
|
||||
@pytest.mark.live_system_guard_bypass # the launcher's group must really be signalled
|
||||
def test_readiness_timeout_terminates_the_launch_it_gave_up_on(in_process_runtime):
|
||||
"""When the launcher misses the readiness deadline start() raises — and must take the launch down with
|
||||
it. It used to leave the launcher running; the child then published DISPLAY/rfb.sock a moment later and
|
||||
a screen nobody asked for (and whose start() had reported failure) stayed up behind a 'running' status."""
|
||||
import time
|
||||
|
||||
scratch = in_process_runtime
|
||||
(scratch / "launcher.sh").write_text(_SLOW_LAUNCHER, encoding="utf-8")
|
||||
sd = runtime.state_dir()
|
||||
with pytest.raises(RuntimeError, match="did not publish"):
|
||||
runtime.start(wait_seconds=0.05)
|
||||
launcher = runtime._recorded_launcher_pid()
|
||||
assert launcher is None or _gone(launcher), "the timed-out launcher must be reaped, not left to publish later"
|
||||
time.sleep(1.5) # past the slow launcher's publish time
|
||||
assert not (sd / "env").exists() and not (sd / "rfb.sock").exists()
|
||||
assert runtime.status().running is False
|
||||
for lock in (scratch / "xlocks").glob(".X*-lock"):
|
||||
assert _gone(int(lock.read_text())), "the launch's X server must die with its launcher"
|
||||
|
||||
@@ -412,6 +412,13 @@ def _spawn_and_wait(sd: Path, num: int, wait_seconds: float) -> DesktopStatus:
|
||||
logger.info("Bot Desktop for profile %s up on :%s", _profile_name(), num)
|
||||
return status()
|
||||
time.sleep(0.1)
|
||||
# Giving up must take the launch down: left alone, the launcher publishes DISPLAY and rfb.sock a moment
|
||||
# later and a screen whose start() reported failure stays up as "running". The launcher is its own
|
||||
# session leader, so its group is exactly this launch (Xvnc, dbus, Xfce) and nothing else.
|
||||
_kill_group_then_wait(proc.pid, proc.pid)
|
||||
proc.wait()
|
||||
for name in ("launcher.pid", "env", "rfb.sock"):
|
||||
(sd / name).unlink(missing_ok=True)
|
||||
raise RuntimeError(f"Bot Desktop did not publish its display within {wait_seconds:.0f}s (see {sd / 'launcher.log'})")
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user