From 6a7a7d893301d3894f0f2eab3bb63d9406c8ae7f Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Fri, 21 Aug 2026 13:04:54 +0200 Subject: [PATCH] Queue behind stronger lockers, single-instance guard, fix budget-crash in fit check Root-caused the 2026-08-21 stuck-lock incident: a manual workout unlocked correctly, but the mechanism behind "locked again seconds later" was a stray workout-locker.service re-arm racing a second process into building a competing lock window -- not a leetcode-guard conflict as first suspected (its timer wasn't due until 13:00 that day). Separately, diet_guard held the global grab for ~3 minutes against a newly-arriving, higher-ranked screen_locker that morning, which just retried and logged instead of making progress. - _build_lock_window now calls gatelock.wait_for_turn before arming, so a stronger incumbent is queued behind instead of raced. - New screen_locker/_instance.py (flock-based, mirrors leetcode_guard/diet_guard's own single-instance guards) stops a second process from ever building a competing window in the first place. - Bumped to gatelock v0.7.1, which adds weaker-holder preemption (SIGTERM, reusing the holder's own clean close() path) so a lower- ranked incumbent can no longer block a stronger claim for minutes. - Fixed a live crash in scripts/verify_screen_fits.py: it read the real log.json, so today's exhausted manual-workout budget (itself caused by the incident's duplicate entry) made the fit-check crash on an unset _mw_error_label. Points at a path that can never exist instead, so the measurement no longer depends on live data. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_018M5gKyYzxWG6QkCaj1wWfk --- pyproject.toml | 2 +- requirements.txt | 2 +- screen_locker/_constants.py | 8 +++- screen_locker/_instance.py | 68 ++++++++++++++++++++++++++++++++++ screen_locker/_window_setup.py | 8 ++++ screen_locker/screen_lock.py | 20 ++++++++++ scripts/verify_screen_fits.py | 12 +++++- 7 files changed, 115 insertions(+), 5 deletions(-) create mode 100644 screen_locker/_instance.py diff --git a/pyproject.toml b/pyproject.toml index 232628e..139d035 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ requires-python = ">=3.10" # crdt-sync: shared CRDT merge scheme + GitHub-Contents-API sync transport, # also used by diet_guard; here for phone-workout GitHub sync (Workstream C) dependencies = [ - "gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock", + "gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.7.1#subdirectory=gatelock", "crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.7.0#subdirectory=crdt-sync", ] diff --git a/requirements.txt b/requirements.txt index ff81626..bd660d7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ bandit>=1.7.0 codespell>=2.2.0 coverage>=7.4.0 crdt-sync @ git+https://github.com/kuhyx/utils@crdt-sync-v0.7.0#subdirectory=crdt-sync -gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.4.0#subdirectory=gatelock +gatelock @ git+https://github.com/kuhyx/utils@gatelock-v0.7.1#subdirectory=gatelock mcp>=2,<3 mypy>=1.8.0 pre-commit>=3.6.0 diff --git a/screen_locker/_constants.py b/screen_locker/_constants.py index 688e0bc..f1a666b 100644 --- a/screen_locker/_constants.py +++ b/screen_locker/_constants.py @@ -4,8 +4,14 @@ from __future__ import annotations from pathlib import Path +from gatelock import default_runtime_dir from gatelock.log_integrity import DEFAULT_HMAC_KEY_FILE +# Ephemeral runtime state (self-clears on reboot, same directory gatelock's +# own Arbiter uses) -- not repo-tracked data, so it does not belong next to +# log.json/sick_history.json below. +INSTANCE_LOCK_FILE = default_runtime_dir() / "screen_locker-instance.lock" + # Single-sourced from gatelock so the literal key path can't drift. HMAC_KEY_FILE = DEFAULT_HMAC_KEY_FILE SICK_LOCKOUT_SECONDS = 120 # base 2 minutes wait when sick (escalates with usage) @@ -87,8 +93,6 @@ MIN_RUN_DISTANCE_KM = 3.5 # inclusive: "as low as 3.5 km" # that one is a deliberate concession to the user, this one corrects a # measurement error. Duration now gets a leeway too, but for the other reason. RUNNERUP_DISTANCE_TOLERANCE = 0.05 -RUNNERUP_ACCEPTED_SPORTS: frozenset[int] = frozenset({0, 3, 5}) -# 0=RUNNING, 3=ORIENTEERING, 5=TREADMILL MAX_CLOCK_SKEW_SECONDS = 300 # 5 minutes max time skew from NTP EARLY_BIRD_START_HOUR = 5 EARLY_BIRD_END_HOUR = 8 diff --git a/screen_locker/_instance.py b/screen_locker/_instance.py new file mode 100644 index 0000000..1deec83 --- /dev/null +++ b/screen_locker/_instance.py @@ -0,0 +1,68 @@ +"""One workout-locker process at a time. + +A stray re-arm (a delayed ``workout-locker.timer`` catch-up landing right on +top of a session that just unlocked, a direct spawn outside the tracked +systemd unit, or any other double-start) used to mean two Tk processes +racing to build a lock window and grab the display. Copied in shape from +``leetcode_guard._instance`` / ``diet_guard._gatelock.acquire_gate_lock``. + +Liveness is the kernel's ``flock``, held for the process lifetime: it is +released on *any* death, including SIGKILL and a crashed X server. No PID +files, no staleness heuristics, no timeouts. +""" + +from __future__ import annotations + +from dataclasses import dataclass +import fcntl +import logging +import os +from typing import IO, TYPE_CHECKING, Final + +_logger: Final = logging.getLogger(__name__) + +if TYPE_CHECKING: + from pathlib import Path + + +@dataclass +class InstanceLock: + """A held single-instance lock.""" + + handle: IO[str] + path: Path + + def release(self) -> None: + """Drop the lock. Idempotent, and safe on a closed handle.""" + try: + self.handle.close() + except OSError as exc: + _logger.warning("could not close the instance lock %s: %s", self.path, exc) + + +def acquire(path: Path) -> InstanceLock | None: + """Take the single-instance lock, or ``None`` if another run holds it. + + Opened ``"a+"`` rather than ``"w"``: ``"w"`` truncates at ``open()`` time, + which happens *before* the lock attempt, so a losing contender would erase + the incumbent's record on its way out. + """ + try: + path.parent.mkdir(parents=True, exist_ok=True) + handle = path.open("a+", encoding="utf-8") + except OSError: + _logger.exception("cannot open the instance lock %s", path) + return None + try: + fcntl.flock(handle.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + except OSError: + _logger.warning( + "another workout-locker run already holds %s -- standing down", path + ) + handle.close() + return None + handle.seek(0) + handle.truncate() + handle.write(str(os.getpid())) + handle.flush() + return InstanceLock(handle=handle, path=path) diff --git a/screen_locker/_window_setup.py b/screen_locker/_window_setup.py index 3ed35e5..7bf0799 100644 --- a/screen_locker/_window_setup.py +++ b/screen_locker/_window_setup.py @@ -17,6 +17,7 @@ from gatelock import ( Arbiter, LockWindow, ScrollableSurface, + wait_for_turn, ) from screen_locker._surface_group import FrameGroup @@ -86,6 +87,13 @@ class WindowSetupMixin: disable_vt=self._colors.resolved_disable_vt(), ) arbiter.publish() + # Published first so anything lower-ranked queues behind us in turn, + # then wait with nothing on screen. Never exits here: a higher-ranked + # locker (wake_alarm) means "later", never "skip" -- mirrors + # leetcode_guard's own fix for the same grab race (gatelock's own + # module docstring: 2026-07-25, and again with diet_guard on + # 2026-08-21). + wait_for_turn(arbiter) arbiter.acquire_holder() lock = LockWindow(self.root, self._colors, hooks=self, arbiter=arbiter) lock.setup() diff --git a/screen_locker/screen_lock.py b/screen_locker/screen_lock.py index 268c0a1..41558f6 100755 --- a/screen_locker/screen_lock.py +++ b/screen_locker/screen_lock.py @@ -18,12 +18,14 @@ from gatelock import ( LockWindow, ) +from screen_locker import _instance from screen_locker._auto_upgrade import AutoUpgradeMixin from screen_locker._constants import ( EARLY_BIRD_END_HOUR, EARLY_BIRD_END_MINUTE, EARLY_BIRD_START_HOUR, HMAC_KEY_FILE, + INSTANCE_LOCK_FILE, MAX_CLOCK_SKEW_SECONDS, MIN_WORKOUT_DURATION_MINUTES, PHONE_PENALTY_DELAY_DEMO, @@ -114,6 +116,23 @@ class ScreenLocker( ) -> None: """Initialize screen locker with optional demo mode.""" _assert_not_under_pytest() + # Guards against a stray re-arm racing a second process into building + # a competing lock window (the actual mechanism behind "unlocked, then + # locked again seconds later" -- see 2026-08-21 incident notes). Not + # released in a try/finally: standing down here calls sys.exit(0) + # immediately, and process exit already drops the flock for free. + instance_lock = _instance.acquire(INSTANCE_LOCK_FILE) + if instance_lock is None: + # _instance.acquire already logged who holds it; standing down + # here (rather than building a second competing window) is the + # fix for the actual mechanism behind "unlocked, then locked + # again seconds later". + _logger.warning( + "standing down: another workout-locker instance already holds %s", + INSTANCE_LOCK_FILE, + ) + sys.exit(0) + self._instance_lock: _instance.InstanceLock = instance_lock script_dir = Path(__file__).resolve().parent self.log_file = script_dir / "log.json" self.verify_only = verify_only @@ -169,6 +188,7 @@ class ScreenLocker( self._lock.close() else: self.root.destroy() + self._instance_lock.release() # The lock screen coming down is the end of enforcement for this run, # so it belongs in the journal next to the decision that raised it — # otherwise "the lock closed" and "the lock never opened" look alike. diff --git a/scripts/verify_screen_fits.py b/scripts/verify_screen_fits.py index cce3d3a..7944b82 100755 --- a/scripts/verify_screen_fits.py +++ b/scripts/verify_screen_fits.py @@ -95,8 +95,18 @@ class ScreenHost( self.root = surface.content.winfo_toplevel() self.demo_mode = False self.verify_only = False + # Deliberately NOT the real log.json: is_budget_exhausted() reads it, + # and the real file's content depends on today's actual usage -- + # exhausting the manual-workout budget in production made this crash + # (paint_manual_form_with_error touches _mw_error_label, which the + # budget-exhausted branch of _show_manual_workout_form never sets). + # A path that can never exist keeps every measured screen's shape + # independent of live data, which is the same guarantee every other + # attribute on this class already gives (see class docstring). self.log_file = ( - Path(__file__).resolve().parent.parent / "screen_locker" / "log.json" + Path(__file__).resolve().parent.parent + / "screen_locker" + / "fit_check_never_exists.json" ) self._colors = LockConfig() self.container = FrameGroup([surface.content])