diff --git a/tools/agent-isolation/gpg-touch-overlay-window-macos.py b/tools/agent-isolation/gpg-touch-overlay-window-macos.py index 53b2f971c..7799e8e3a 100644 --- a/tools/agent-isolation/gpg-touch-overlay-window-macos.py +++ b/tools/agent-isolation/gpg-touch-overlay-window-macos.py @@ -265,8 +265,27 @@ def send(ret, *argtypes): ) -def build_window(): +def take_back_signals(stopping): + """Route termination signals to the flag ``watch_for_stop`` polls. + + Only once the root exists: Aqua Tk's initialisation installs its own + SIGINT/SIGHUP/SIGTERM handler, replacing anything set before + ``tk.Tk()``. That handler calls ``Tcl_Exit`` from inside the signal, + which is not async-signal-safe — a SIGTERM landing mid-redraw (the + pulse repaints every ``PULSE_MS``) tears the windows down while + CoreAnimation's backing-store lock is held by the interrupted draw, + and the destroy blocks on that same lock. The window then hangs on + screen, deadlocked on its own main thread, until force-quit. + Python's handler only sets a flag, so nothing unsafe runs in signal + context. + """ + for sig in (signal.SIGTERM, signal.SIGINT, signal.SIGHUP): + signal.signal(sig, lambda *_: stopping.append(True)) + + +def build_window(stopping): root = tk.Tk() + take_back_signals(stopping) root.title(TITLE) root.configure(bg=BG_HEX) @@ -349,10 +368,7 @@ def watch_for_stop(root, stopping): def main(): stopping = [] - signal.signal(signal.SIGTERM, lambda *_: stopping.append(True)) - signal.signal(signal.SIGINT, lambda *_: stopping.append(True)) - - root = build_window() + root = build_window(stopping) watch_for_stop(root, stopping) root.mainloop() return 0 diff --git a/tools/agent-isolation/tests/test_gpg_touch_overlay.py b/tools/agent-isolation/tests/test_gpg_touch_overlay.py index 92495f0d7..3f9082c8d 100644 --- a/tools/agent-isolation/tests/test_gpg_touch_overlay.py +++ b/tools/agent-isolation/tests/test_gpg_touch_overlay.py @@ -1248,3 +1248,54 @@ def test_the_aqua_window_closes_when_it_loses_the_keyboard() -> None: f"keyboard; it binds only {sorted(bound)}" ) assert "" in bound, "the Aqua overlay no longer closes on Esc" + + +def test_the_aqua_window_takes_termination_signals_back_from_tk() -> None: + """The watcher's SIGTERM must reach the flag, never Tk's own handler. + + Aqua Tk's initialisation installs a SIGINT/SIGHUP/SIGTERM handler that + calls ``Tcl_Exit`` from inside the signal. Landing mid-redraw it tears + the windows down while the interrupted draw holds CoreAnimation's + backing-store lock, and the teardown blocks on that lock: the overlay + hangs on screen until force-quit. Python handlers installed *before* + ``tk.Tk()`` are silently replaced, so the order is the whole fix — + ``take_back_signals`` has to run after the root exists, and nothing + may install a handler ahead of it. + """ + window = SCRIPT.parent / "gpg-touch-overlay-window-macos.py" + tree = ast.parse(window.read_text()) + functions = { + node.name: node for node in tree.body if isinstance(node, ast.FunctionDef) + } + + def calls(node: ast.AST) -> list[str]: + found: list[tuple[int, str]] = [] + for sub in ast.walk(node): + if isinstance(sub, ast.Call): + func = sub.func + if isinstance(func, ast.Attribute): + found.append((sub.lineno, func.attr)) + elif isinstance(func, ast.Name): + found.append((sub.lineno, func.id)) + return [name for _, name in sorted(found)] + + build = calls(functions["build_window"]) + assert "Tk" in build and "take_back_signals" in build, ( + "build_window no longer creates the root and then takes the signals back" + ) + assert build.index("take_back_signals") > build.index("Tk"), ( + "signal handlers installed before tk.Tk() are replaced by Tk's own, " + "which deadlocks the window when SIGTERM lands mid-draw" + ) + assert "signal" not in calls(functions["main"]), ( + "main() installs a signal handler before tk.Tk() exists; Tk replaces it" + ) + + handled = { + sub.attr + for sub in ast.walk(functions["take_back_signals"]) + if isinstance(sub, ast.Attribute) and sub.attr.startswith("SIG") + } + assert {"SIGTERM", "SIGINT", "SIGHUP"} <= handled, ( + f"Tk's handler still owns {sorted({'SIGTERM', 'SIGINT', 'SIGHUP'} - handled)}" + )