From ca3113ac7b01df380db503f4ef429d0c0986e5ce Mon Sep 17 00:00:00 2001 From: Rishet Mehra Date: Sat, 25 Jul 2026 01:55:16 +0530 Subject: [PATCH] fix(hooks): arm the rebuild timeout without SIGALRM on Windows The GRAPHIFY_REBUILD_TIMEOUT watchdog in both embedded rebuild bodies was guarded by hasattr(signal, 'SIGALRM') with no else branch, so on Windows it never armed and a hung rebuild survived indefinitely -- the exact tail case the #791 timeout was added to catch. Fall back to a daemon threading.Timer that prints the same message and calls os._exit(1). The hard exit is deliberate: the process is already stuck, so a clean shutdown may itself be blocked, and _rebuild_lock degrades to a no-op yield on platforms without fcntl, so there is no lock to leave stale. --- graphify/hooks.py | 32 ++++++++++++++++++++++++-------- tests/test_hooks.py | 20 ++++++++++++++++++++ 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/graphify/hooks.py b/graphify/hooks.py index cbaae1b4..455c70a4 100644 --- a/graphify/hooks.py +++ b/graphify/hooks.py @@ -104,7 +104,7 @@ fi # double-quote, $, backtick or backslash characters: it is carried inside a # shell double-quoted `-c "..."` argument (see _detached_launch). _REBUILD_BODY_COMMIT = """\ -import os, signal, sys +import os, signal, sys, threading from pathlib import Path changed_raw = os.environ.get('GRAPHIFY_CHANGED', '') @@ -119,9 +119,17 @@ try: from graphify.watch import _rebuild_code, _apply_resource_limits _apply_resource_limits() _timeout = int(os.environ.get('GRAPHIFY_REBUILD_TIMEOUT', '600')) - if _timeout > 0 and hasattr(signal, 'SIGALRM'): - signal.signal(signal.SIGALRM, lambda *_: (_ for _ in ()).throw(TimeoutError(f'graphify rebuild exceeded {_timeout}s'))) - signal.alarm(_timeout) + if _timeout > 0: + if hasattr(signal, 'SIGALRM'): + signal.signal(signal.SIGALRM, lambda *_: (_ for _ in ()).throw(TimeoutError(f'graphify rebuild exceeded {_timeout}s'))) + signal.alarm(_timeout) + else: + def _bail(): + print(f'[graphify hook] graphify rebuild exceeded {_timeout}s', flush=True) + os._exit(1) + _watchdog = threading.Timer(_timeout, _bail) + _watchdog.daemon = True + _watchdog.start() _force = os.environ.get('GRAPHIFY_FORCE', '').lower() in ('1', 'true', 'yes') _root = Path('.') _out = os.environ.get('GRAPHIFY_OUT', 'graphify-out') @@ -153,13 +161,21 @@ except Exception as exc: _REBUILD_BODY_CHECKOUT = """\ from graphify.watch import _rebuild_code, _apply_resource_limits from pathlib import Path -import os, signal, sys +import os, signal, sys, threading try: _apply_resource_limits() _timeout = int(os.environ.get('GRAPHIFY_REBUILD_TIMEOUT', '600')) - if _timeout > 0 and hasattr(signal, 'SIGALRM'): - signal.signal(signal.SIGALRM, lambda *_: (_ for _ in ()).throw(TimeoutError(f'graphify rebuild exceeded {_timeout}s'))) - signal.alarm(_timeout) + if _timeout > 0: + if hasattr(signal, 'SIGALRM'): + signal.signal(signal.SIGALRM, lambda *_: (_ for _ in ()).throw(TimeoutError(f'graphify rebuild exceeded {_timeout}s'))) + signal.alarm(_timeout) + else: + def _bail(): + print(f'[graphify hook] graphify rebuild exceeded {_timeout}s', flush=True) + os._exit(1) + _watchdog = threading.Timer(_timeout, _bail) + _watchdog.daemon = True + _watchdog.start() _force = os.environ.get('GRAPHIFY_FORCE', '').lower() in ('1', 'true', 'yes') # post-checkout: branch switch can touch arbitrary files; full rebuild path # (no changed_paths) is correct here. The flock inside _rebuild_code still diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 2f0fc588..f21b7e31 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -330,6 +330,26 @@ def test_rebuild_bodies_with_graphify_root_are_valid_python(): ast.parse(body) +@pytest.mark.parametrize( + "name,body", + [("post-commit", _REBUILD_BODY_COMMIT), ("post-checkout", _REBUILD_BODY_CHECKOUT)], +) +def test_rebuild_bodies_arm_a_timeout_without_sigalrm(name, body): + """Windows has no signal.SIGALRM, so the #791 rebuild timeout never armed + there at all (#2148). The fallback has to sit in the else-branch of the + SIGALRM check rather than merely appear somewhere in the body, so that a + watchdog firing unconditionally or on every platform still fails here.""" + fallbacks = [ + node.orelse + for node in ast.walk(ast.parse(body)) + if isinstance(node, ast.If) and "'SIGALRM'" in ast.dump(node.test) and node.orelse + ] + assert fallbacks, f"{name} has no else-branch for the missing-SIGALRM case (#2148)" + dumped = "".join(ast.dump(stmt) for stmt in fallbacks[0]) + assert "attr='Timer'" in dumped, f"{name} fallback does not arm a threading.Timer (#2148)" + assert "attr='_exit'" in dumped, f"{name} fallback does not kill the stuck rebuild (#2148)" + + def test_detached_launch_targets_graphify_python(): """The launcher must run via the resolved $GRAPHIFY_PYTHON, not a bare `python`, so it uses the same interpreter the detection block selected."""