diff --git a/screen_locker/_workout_handoff.py b/screen_locker/_workout_handoff.py index bbe97ce..78a5016 100644 --- a/screen_locker/_workout_handoff.py +++ b/screen_locker/_workout_handoff.py @@ -65,13 +65,15 @@ def start_workout(lock: LockWindow) -> str: so a failure has to come back as a message rather than a traceback that would leave the grab in an unknown state. - Blocks until the app exits, which stops the Tk event loop for the whole - workout. That is deliberate rather than merely tolerated: the Flutter - window is covering the screen, and the alternative -- pumping Tk - underneath -- is what would let the watchdog tick and take the screen back - mid-set. The cost is that a monitor hotplug during the workout is not - handled until the app exits, when ``reacquire`` re-runs ``grab_input()`` - and the watchdog re-covers every live output. + Blocks until the app exits, so it is only safe for a caller with no event + loop to starve. Behind a Tk button use :class:`WorkoutSession` instead: + blocking there freezes the loop, which makes every binding on the lock -- + including the escapes -- inert until the app exits. + + An earlier version of this docstring claimed the blocking was deliberate, + to stop the recovery watchdog ticking mid-workout. That was wrong: + ``release`` stops the watchdog first, and ``stop()`` cancels its timers + outright, so there are no ticks to race with either way. """ result = launch_workout_app(lock_grab_handoff(lock)) if not result.launched: diff --git a/screen_locker/_workout_session.py b/screen_locker/_workout_session.py new file mode 100644 index 0000000..9b1f741 --- /dev/null +++ b/screen_locker/_workout_session.py @@ -0,0 +1,165 @@ +"""Running the workout app without freezing the lock's event loop. + +``launch_workout_app`` blocks until the app exits, which is fine for a +headless caller but wrong behind a Tk button: while it blocks, the event loop +is not running, so key bindings and buttons on the lock itself are inert. In +the demo lock -- whose whole promise is that Escape always works -- that +turned a "safe" screen into one that needed a hard reboot to leave. + +So the phases are driven by ``after`` instead, on the main thread. Every Tk +call (``grab_release``, ``grab_input``) stays there, which a worker thread +could not guarantee, and the loop keeps pumping the entire time the app is up. + +This does not weaken the production handoff. The grab is still released only +after the ready line, and the recovery watchdog is still stopped first -- and +because ``stop()`` cancels its timers outright, there are no ticks to race +with while the app runs. +""" + +from __future__ import annotations + +import logging +import subprocess +from typing import TYPE_CHECKING + +from screen_locker._workout_app import ProcessHooks, workout_app_binary +from screen_locker._workout_ready import READY_MARKER + +if TYPE_CHECKING: + from collections.abc import Callable + from pathlib import Path + from typing import IO, Protocol + + from screen_locker._workout_app import GrabHandoff + + class _Child(Protocol): + """The half of ``subprocess.Popen`` this module drives.""" + + stdout: IO[str] | None + returncode: int | None + + def poll(self) -> int | None: ... + + def terminate(self) -> None: ... + + +__all__ = ["WorkoutSession"] + +_logger = logging.getLogger(__name__) + +# How often to look at the app while it runs. Short enough that the lock comes +# back promptly when the app exits, long enough not to spin the event loop. +_POLL_MS = 100 + +# 600 polls x 100ms = 60s, matching the blocking supervisor's own deadline. +_READY_TIMEOUT_POLLS = 600 + + +class WorkoutSession: + """One supervised run of the workout app, driven by the Tk event loop. + + ``on_status`` is called with a human-readable line at each transition, so a + failure is visible on the lock screen rather than only in the journal. + """ + + def __init__( + self, + handoff: GrabHandoff, + *, + after: Callable[[int, Callable[[], None]], object], + on_status: Callable[[str], None], + binary: Path | None = None, + hooks: ProcessHooks | None = None, + ) -> None: + self._handoff = handoff + self._after = after + self._on_status = on_status + self._binary = binary + self._hooks = hooks if hooks is not None else ProcessHooks() + self._remaining = _READY_TIMEOUT_POLLS + self._child: _Child | None = None + self._seen = "" + self._released = False + + def start(self) -> None: + """Spawn the app; the rest happens on later event-loop turns.""" + app = self._binary if self._binary is not None else workout_app_binary() + if app is None: + self._fail("the desktop workout app is not built; run flutter build linux") + return + self._child = self._hooks.popen( + [str(app), "--lock-mode"], + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + bufsize=1, + ) + self._on_status("Starting the workout app…") + self._after(_POLL_MS, self._await_ready) + + def _await_ready(self) -> None: + """Watch for the ready line without ever blocking the loop.""" + child = self._child + stdout = getattr(child, "stdout", None) + if stdout is None: + self._fail("the workout app exited before signalling ready") + return + # Zero timeout: ask what is there right now and return to the loop. + readable, _, _ = self._hooks.selector([stdout], [], [], 0) + if readable: + chunk = _read_available(stdout) + if not chunk: + self._fail("the workout app exited before signalling ready") + return + self._seen += chunk + if READY_MARKER in self._seen: + self._release_and_watch() + return + self._remaining -= 1 + if self._remaining <= 0: + # Never release to a window that may not be up. + child.terminate() + self._fail("the workout app never signalled ready in time") + return + self._after(_POLL_MS, self._await_ready) + + def _release_and_watch(self) -> None: + self._handoff.release() + self._released = True + self._on_status("The workout app has the screen.") + self._after(_POLL_MS, self._watch_until_exit) + + def _watch_until_exit(self) -> None: + if self._child.poll() is None: + self._after(_POLL_MS, self._watch_until_exit) + return + self.finish() + + def finish(self) -> None: + """Take the screen back. Idempotent, so an early abort is safe.""" + if self._released: + self._released = False + self._handoff.reacquire() + code = getattr(self._child, "returncode", None) + _logger.warning("The workout app exited with code %s.", code) + self._on_status("Workout app closed — checking whether it counted…") + + def abort(self) -> None: + """Kill the app and restore the lock, whatever phase we are in.""" + child = self._child + if child is not None and child.poll() is None: + child.terminate() + _logger.warning("Workout app terminated by an explicit abort.") + self.finish() + + def _fail(self, reason: str) -> None: + _logger.error("Could not run the PC workout: %s", reason) + self._on_status(f"Could not start the workout: {reason}") + + +def _read_available(stream: IO[str]) -> str: + """Read what the pipe holds now; see ``_workout_ready`` for the why.""" + buffer = getattr(stream, "buffer", None) + if buffer is None: + return stream.readline() + return buffer.read1().decode("utf-8", "replace") diff --git a/screen_locker/tests/test_workout_session.py b/screen_locker/tests/test_workout_session.py new file mode 100644 index 0000000..6db8eb0 --- /dev/null +++ b/screen_locker/tests/test_workout_session.py @@ -0,0 +1,209 @@ +"""Tests for running the workout app without freezing the lock's event loop. + +The invariant these defend: no phase blocks. Every wait is an ``after`` +callback, because a blocked event loop makes the lock's own escapes inert -- +which is what once forced a hard reboot out of a "safe" demo. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import TYPE_CHECKING + +from screen_locker._workout_app import GrabHandoff, ProcessHooks +from screen_locker._workout_ready import READY_MARKER +from screen_locker._workout_session import WorkoutSession + +if TYPE_CHECKING: + from collections.abc import Callable + + +class _FakeAfter: + """A drainable stand-in for ``root.after``.""" + + def __init__(self) -> None: + self.queue: list[Callable[[], None]] = [] + + def __call__(self, _ms: int, callback: Callable[[], None]) -> object: + self.queue.append(callback) + return object() + + def drain(self, limit: int = 5000) -> None: + for _ in range(limit): + if not self.queue: + return + self.queue.pop(0)() + + +class _FakeStdout: + def __init__(self, chunks: list[str]) -> None: + self._chunks = chunks + + def readline(self) -> str: + return self._chunks.pop(0) if self._chunks else "" + + +class _FakeChild: + def __init__(self, chunks: list[str], *, exits_after: int = 1) -> None: + self.stdout = _FakeStdout(chunks) + self.returncode: int | None = None + self.terminated = False + self._polls = 0 + self._exits_after = exits_after + + def poll(self) -> int | None: + self._polls += 1 + if self._polls >= self._exits_after: + self.returncode = 0 + return 0 + return None + + def terminate(self) -> None: + self.terminated = True + self.returncode = -15 + + +def _session(child: _FakeChild, after: _FakeAfter, calls: list[str]): + return WorkoutSession( + GrabHandoff( + release=lambda: calls.append("release"), + reacquire=lambda: calls.append("reacquire"), + ), + after=after, + on_status=lambda text: calls.append(f"status:{text[:20]}"), + binary=Path("/nonexistent/workout_app"), + hooks=ProcessHooks( + popen=lambda *_a, **_k: child, + selector=lambda r, _w, _x, _t: (r, [], []), + ), + ) + + +class TestWorkoutSession: + """Each phase hands control back to the event loop.""" + + def test_releases_the_grab_only_after_the_ready_line(self) -> None: + after, calls = _FakeAfter(), [] + child = _FakeChild([f"{READY_MARKER}\n"], exits_after=2) + _session(child, after, calls).start() + assert "release" not in calls # nothing released synchronously + after.drain() + assert "release" in calls + assert calls.index("release") < calls.index("reacquire") + + def test_never_releases_when_the_app_dies_before_ready(self) -> None: + after, calls = _FakeAfter(), [] + _session(_FakeChild([]), after, calls).start() + after.drain() + assert "release" not in calls + + def test_reacquires_when_the_app_exits(self) -> None: + after, calls = _FakeAfter(), [] + child = _FakeChild([f"{READY_MARKER}\n"], exits_after=3) + _session(child, after, calls).start() + after.drain() + assert calls[-2] == "reacquire" + assert "Workout app closed" in calls[-1] + + def test_abort_terminates_the_app_and_restores_the_lock(self) -> None: + """The escape hatch: usable at any point, from outside X.""" + after, calls = _FakeAfter(), [] + child = _FakeChild([f"{READY_MARKER}\n"], exits_after=99) + session = _session(child, after, calls) + session.start() + for _ in range(3): + if after.queue: + after.queue.pop(0)() + session.abort() + assert child.terminated + assert "reacquire" in calls + + def test_abort_before_release_does_not_reacquire(self) -> None: + """Nothing was lent, so there is nothing to take back.""" + after, calls = _FakeAfter(), [] + session = _session(_FakeChild([], exits_after=99), after, calls) + session.start() + session.abort() + assert "reacquire" not in calls + + def test_times_out_without_ever_blocking(self) -> None: + after, calls = _FakeAfter(), [] + child = _FakeChild(["noise\n"] * 10, exits_after=99) + session = _session(child, after, calls) + session._remaining = 3 + session.start() + after.drain() + assert child.terminated + assert "release" not in calls + + def test_says_so_when_the_app_is_not_built(self) -> None: + after, calls = _FakeAfter(), [] + session = WorkoutSession( + GrabHandoff(release=lambda: None, reacquire=lambda: None), + after=after, + on_status=calls.append, + hooks=ProcessHooks(popen=lambda *_a, **_k: None), + ) + # Force the unbuilt case regardless of what this machine has built. + session._binary = None + monkey = "screen_locker._workout_session.workout_app_binary" + import unittest.mock + + with unittest.mock.patch(monkey, return_value=None): + session.start() + assert any("not built" in c for c in calls) + + def test_handles_a_missing_stdout_pipe(self) -> None: + after, calls = _FakeAfter(), [] + child = _FakeChild([]) + child.stdout = None + _session(child, after, calls).start() + after.drain() + assert "release" not in calls + + +class TestSessionEdges: + """Paths a fake pipe or a happy path does not reach.""" + + def test_keeps_waiting_when_the_pipe_has_nothing_yet(self) -> None: + """A quiet app is not a dead app; poll again, never block.""" + after, calls = _FakeAfter(), [] + child = _FakeChild([f"{READY_MARKER}\n"], exits_after=99) + session = WorkoutSession( + GrabHandoff( + release=lambda: calls.append("release"), + reacquire=lambda: calls.append("reacquire"), + ), + after=after, + on_status=lambda _t: None, + binary=Path("/nonexistent/workout_app"), + hooks=ProcessHooks( + popen=lambda *_a, **_k: child, + # Never readable: the app is up but silent. + selector=lambda _r, _w, _x, _t: ([], [], []), + ), + ) + session._remaining = 2 + session.start() + after.drain() + assert "release" not in calls + assert child.terminated + + def test_abort_with_no_child_is_harmless(self) -> None: + after, calls = _FakeAfter(), [] + session = _session(_FakeChild([]), after, calls) + session.abort() # never started + assert len(calls) == 1 + assert "Workout app closed" in calls[0] + + def test_reads_a_real_pipe_without_waiting_for_a_newline(self) -> None: + """The production path: a text stream wrapping a binary buffer.""" + import os + + from screen_locker._workout_session import _read_available + + read_fd, write_fd = os.pipe() + with os.fdopen(read_fd, "r") as reader, os.fdopen(write_fd, "w") as writer: + writer.write("WORKOUT_") + writer.flush() + assert _read_available(reader) == "WORKOUT_" diff --git a/scripts/demo_workout_handoff.py b/scripts/demo_workout_handoff.py index 7623c28..23da203 100644 --- a/scripts/demo_workout_handoff.py +++ b/scripts/demo_workout_handoff.py @@ -1,15 +1,35 @@ """Try the workout-app handoff by hand, on a lock you can always escape. -Deliberately NOT the production lock screen. This builds a demo-mode lock -- -``grab="local"``, VT switching left enabled -- so a wedged Flutter app can -never trap you: Ctrl+Alt+F2, the window manager, and the Escape binding below -all still work. Production uses a global grab and disables VT switching, which -is exactly what makes it unsafe to poke at by hand. +Deliberately NOT the production lock screen: ``grab="local"`` and VT switching +left enabled, so nothing here can outlive a Ctrl+Alt+F2. -One consequence worth knowing before you read anything into a green run: under -a local grab gatelock's watchdog early-returns and never re-takes the grab, so -this exercises the launch, the ready handshake and the reacquire, but NOT the -1000ms steal-back that the global grab makes possible. +The escapes are the point, and the first version of this script got them +wrong in two independent ways -- it promised three and delivered none, and the +only way out was a hard reboot. Both are fixed here, and both are worth +knowing about because only one of them was a demo-only problem: + +1. ``launch_workout_app`` blocks until the app exits. Called straight from a + button, it froze the Tk event loop, so the Escape binding and the close + button could not fire. The run is driven by :class:`WorkoutSession` now, + which uses ``after`` and keeps the loop pumping. +2. The Flutter app takes its OWN seat grab (``gdk_seat_grab`` with + ``GDK_SEAT_CAPABILITY_ALL``) and never releases it. That is correct for + production and fatal for a demo: while the app is up, X delivers keyboard + input to it, so no Tk binding of ours can fire no matter how healthy the + event loop is. So the escape hatch below is deliberately NOT a key binding + -- it is a file the app cannot intercept. + +While the workout app is on screen, escape it from any other terminal or VT: + + touch ~/.cache/stop-workout-demo + +A watchdog polls that path and terminates the app, which hands the screen +back. Ctrl+Alt+F2 also still works, because neither this script nor +``enter_lock_mode`` disables VT switching. + +One thing a green run here does NOT prove: under a local grab gatelock's +watchdog early-returns and never re-takes the grab, so the 1000ms steal-back +that makes ``_recovery.stop()`` load-bearing in production is not exercised. python3 scripts/demo_workout_handoff.py """ @@ -26,7 +46,12 @@ sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) from gatelock import LockConfig, LockWindow from screen_locker._workout_app import workout_app_binary -from screen_locker._workout_handoff import start_workout +from screen_locker._workout_handoff import lock_grab_handoff +from screen_locker._workout_session import WorkoutSession + +# Polled while the app holds the seat grab; the one escape X cannot swallow. +# Under $HOME rather than /tmp: predictable for the user, and not world-writable. +ESCAPE_FILE = Path.home() / ".cache" / "stop-workout-demo" class _DemoHooks: @@ -35,6 +60,7 @@ class _DemoHooks: def __init__(self) -> None: self.status: tk.Label | None = None self.lock: LockWindow | None = None + self.session: WorkoutSession | None = None def build_surface(self, parent: tk.Misc, _surface: object) -> None: tk.Label( @@ -46,7 +72,10 @@ class _DemoHooks: ).pack(pady=(120, 8)) self.status = tk.Label( parent, - text="Press “Start workout” to hand this screen to the app.", + text=( + "Press “Start workout”. To escape while the app holds the " + f"screen, run: touch {ESCAPE_FILE}" + ), font=("TkDefaultFont", 12), bg="#1B1D21", fg="#9AA0A6", @@ -73,11 +102,29 @@ class _DemoHooks: def _start(self) -> None: if self.lock is None or self.status is None: # pragma: no cover return - self.status.config(text="Launching… the screen is handed over shortly.") - self.status.update_idletasks() - # Blocks for the whole workout, by design -- see start_workout's - # docstring. The demo window stops repainting until the app exits. - self.status.config(text=start_workout(self.lock)) + ESCAPE_FILE.unlink(missing_ok=True) + self.session = WorkoutSession( + lock_grab_handoff(self.lock), + after=self.lock.root.after, + on_status=self._set_status, + ) + self.session.start() + self._poll_escape_file() + + def _set_status(self, text: str) -> None: + if self.status is not None: # pragma: no branch + self.status.config(text=text) + + def _poll_escape_file(self) -> None: + """The escape the Flutter app's seat grab cannot intercept.""" + if self.session is None or self.lock is None: # pragma: no cover + return + if ESCAPE_FILE.exists(): + ESCAPE_FILE.unlink(missing_ok=True) + self.session.abort() + self.session = None + return + self.lock.root.after(200, self._poll_escape_file) def _close(self) -> None: if self.lock is not None: # pragma: no cover