From ed948388db4c871ea614cbba6b344a5e5bc73573 Mon Sep 17 00:00:00 2001 From: ayushcodes10 Date: Mon, 7 Sep 2026 17:18:12 +0530 Subject: [PATCH] Kill workers inside the SIGALRM handler too, before it raises The no-SIGALRM fallback killing workers before os._exit does not cover the primary path: on POSIX, where SIGALRM is available, a TimeoutError raised while the main thread is waiting inside the ProcessPoolExecutor with-block propagates straight through that block's own __exit__, which calls shutdown(wait=True) and blocks until every worker exits. Forever, for a worker stuck in a C-level call the alarm firing does nothing to stop, the exact shape #3341 was. The except TimeoutError handler that would call sys.exit is never reached, so the configured timeout bounds nothing in that case. Reproduced directly: a worker in an unconditional loop, a SIGALRM after 2s, and the process was still running 15s later. Kills any live multiprocessing.active_children() inside the signal handler itself, before it raises, so by the time the exception reaches shutdown(wait=True) there is nothing left to wait for. Moved the handler from an inline lambda (which cannot contain a statement, only expressions) to a named function, matching the no-SIGALRM fallback's own style. Fixes #3396. --- graphify/hooks.py | 26 +++++++++++++++++++++-- tests/test_hooks.py | 50 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/graphify/hooks.py b/graphify/hooks.py index a94f932ee..aaae7d67d 100644 --- a/graphify/hooks.py +++ b/graphify/hooks.py @@ -151,7 +151,18 @@ try: _timeout = int(os.environ.get('GRAPHIFY_REBUILD_TIMEOUT', '600')) if _timeout > 0: if hasattr(signal, 'SIGALRM'): - signal.signal(signal.SIGALRM, lambda *_: (_ for _ in ()).throw(TimeoutError(f'graphify rebuild exceeded {_timeout}s'))) + def _sigalrm_bail(*_a): + # Killing here, before the exception unwinds, matters: once + # TimeoutError starts propagating it passes straight through + # the ProcessPoolExecutor with-block's own __exit__, which + # calls shutdown(wait=True) and blocks until every worker + # exits -- forever, for a worker stuck the way #3341 was, + # since the alarm firing never actually stops it. Killing the + # workers first means shutdown has nothing left to wait for. + for _child in multiprocessing.active_children(): + _child.kill() + raise TimeoutError(f'graphify rebuild exceeded {_timeout}s') + signal.signal(signal.SIGALRM, _sigalrm_bail) signal.alarm(_timeout) else: def _bail(): @@ -199,7 +210,18 @@ try: _timeout = int(os.environ.get('GRAPHIFY_REBUILD_TIMEOUT', '600')) if _timeout > 0: if hasattr(signal, 'SIGALRM'): - signal.signal(signal.SIGALRM, lambda *_: (_ for _ in ()).throw(TimeoutError(f'graphify rebuild exceeded {_timeout}s'))) + def _sigalrm_bail(*_a): + # Killing here, before the exception unwinds, matters: once + # TimeoutError starts propagating it passes straight through + # the ProcessPoolExecutor with-block's own __exit__, which + # calls shutdown(wait=True) and blocks until every worker + # exits -- forever, for a worker stuck the way #3341 was, + # since the alarm firing never actually stops it. Killing the + # workers first means shutdown has nothing left to wait for. + for _child in multiprocessing.active_children(): + _child.kill() + raise TimeoutError(f'graphify rebuild exceeded {_timeout}s') + signal.signal(signal.SIGALRM, _sigalrm_bail) signal.alarm(_timeout) else: def _bail(): diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 69e14863a..d8eb4573d 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py @@ -412,6 +412,56 @@ def test_rebuild_bodies_kill_children_before_os_exit(name, body): ) +@pytest.mark.parametrize( + "name,body", + [("post-commit", _REBUILD_BODY_COMMIT), ("post-checkout", _REBUILD_BODY_CHECKOUT)], +) +def test_sigalrm_handler_kills_children_before_raising(name, body): + """The no-SIGALRM fallback killing children before os._exit is not enough: + on POSIX, where SIGALRM IS available, a TimeoutError raised while the main + thread is waiting inside the ProcessPoolExecutor with-block propagates + straight through that block's own __exit__, which calls + shutdown(wait=True) and blocks until every worker exits -- forever, for a + worker stuck in a C-level call the alarm firing does nothing to stop + (reproduced: a worker in an unconditional loop left the process still + running 15s after a 2s alarm). The except TimeoutError handler is never + reached, so the timeout provides no bound at all in that case. Killing + workers INSIDE the signal handler, before it raises, means shutdown has + nothing left to wait for by the time the exception reaches it.""" + handler_def = next( + n for n in ast.walk(ast.parse(body)) + if isinstance(n, ast.FunctionDef) and n.name == "_sigalrm_bail" + ) + dumped = "".join(ast.dump(stmt) for stmt in handler_def.body) + assert "attr='active_children'" in dumped, ( + f"{name} SIGALRM handler does not enumerate live workers (#3397 follow-up)" + ) + assert "attr='kill'" in dumped, ( + f"{name} SIGALRM handler does not SIGKILL workers (#3397 follow-up)" + ) + + def _stmt_index_calling(attr: str) -> int: + for i, stmt in enumerate(handler_def.body): + if any( + isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) and n.func.attr == attr + for n in ast.walk(stmt) + ): + return i + raise AssertionError(f"{name}: no statement in _sigalrm_bail calls .{attr}(...)") + kill_idx = _stmt_index_calling("kill") + raise_idx = next( + i for i, stmt in enumerate(handler_def.body) if isinstance(stmt, ast.Raise) + ) + assert kill_idx < raise_idx, ( + f"{name} raises TimeoutError before killing workers, defeating the fix" + ) + # signal.signal must be wired to this handler, not still the old inline + # lambda that only threw the exception. + assert re.search(r"signal\.signal\(signal\.SIGALRM,\s*_sigalrm_bail\)", body), ( + f"{name} does not register _sigalrm_bail as the SIGALRM handler" + ) + + 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."""