Skip to content

fix[next]: exit compile-pool workers when their parent dies - #2870

Draft
havogt wants to merge 1 commit into
GridTools:mainfrom
havogt:pool-leak
Draft

fix[next]: exit compile-pool workers when their parent dies#2870
havogt wants to merge 1 commit into
GridTools:mainfrom
havogt:pool-leak

Conversation

@havogt

@havogt havogt commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Description

The build pool's spawn workers were only ever stopped by the atexit-registered reset_default_runner. A parent that ends without interpreter finalization — an embedding host exiting (ICON via py2fgen), os._exit, a SIGKILL from a timeout or systemd-oomd — left them blocked on the call queue forever, together with the resource_tracker. On an ICON run this is 12 orphans per rank surviving the Fortran host; on a laptop they keep adding to memory pressure.

Root cause in the stdlib: every worker also holds the write end of the pool's call queue, so call_queue.get() never sees end-of-file, and the only exit path is the None sentinel the parent's shutdown() sends. The resource_tracker lives as long as the workers because they hold its pipe's write end too.

Fix: _pool_worker_initializer starts a daemon thread that blocks on multiprocessing.parent_process().join() — the parent sentinel spawn already hands each child, which fires on any parent death — and os._exits when it returns. Portable, event-driven, process-scoped. PR_SET_PDEATHSIG was rejected: it tracks the thread that spawned the worker, and since 3.12 the executor spawns from whichever thread calls submit, so a worker spawned from a short-lived thread would die with it (measured: BrokenProcessPool). The atexit ordering against the weakref.finalize cache cleanup is unchanged.

The build pool's spawn workers were only ever stopped by the atexit-registered
shutdown, so a parent that ends without interpreter finalization (an embedding
host exiting, os._exit, SIGKILL) left them blocked on the call queue forever,
together with the resource tracker. Each worker now watches the parent sentinel
multiprocessing already gives it and exits when the parent is gone.

Claude-Session: https://claude.ai/code/session_014MkAWwrEgFrZddvzPG2776

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Abrupt cross-platform process termination semantics warrant final human validation despite the focused tests.

Pull request overview

Prevents orphaned compile-pool workers and resource trackers when their parent terminates abruptly.

Changes:

  • Adds a daemon watchdog that exits workers when the parent dies.
  • Adds tests covering os._exit and forced termination.
File summaries
File Description
src/gt4py/next/otf/runners.py Implements parent-death monitoring.
tests/next_tests/unit_tests/otf_tests/test_runners.py Verifies orphan cleanup behavior.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@havogt havogt left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated independent review (Claude), posted at the author's request.

Verdict: the fix is correct and I found no blocker. The parent sentinel is the right primitive, the daemon thread cannot fire while the parent is alive, and the worst thing an os._exit mid-compile can leave behind is a cache entry the next process rebuilds. Everything below ran on Linux, Python 3.12.12 and 3.14.0, CPU only.

Findings, by severity

  1. Low, new with this change (measured). After the worker _exits, the build step it was waiting on (sh -c c++ … from compiledb.py, or cmake/ninja for the compile-commands prototype) keeps running to completion outside the build-folder lock: the lock is a flock on an fd the worker holds, subprocess runs with close_fds=True, so the grandchild never held it and it dies with the worker. A second process that starts the same build within that window shares the folder with the orphaned step. I measured two cmake and two ninja overlapping in one compile_commands_cache_* folder for about 5 s; all four immediate reruns still succeeded. Before this change the orphaned worker finished the build under the lock and wrote COMPILED; now a rerun always rebuilds once, and can fail loudly (CompilationError/ImportError) if it collides with the orphaned step. It cannot leave a trusted torn entry, see below.
    My view of the remedy: document the window in the PR description and leave it. The failure modes this PR is for (an OOM killer or a scheduler killing the whole process tree) leave no orphaned build step at all. Closing it would mean the worker killing its own build subprocess before _exit (registering the Popen in cmake.py/compiledb.py for the watcher thread; dace's internal cmake calls would stay uncovered) or PR_SET_PDEATHSIG on those subprocesses, which is Linux-only.

  2. Low, pre-existing behaviour, not a defect of this change (measured). A fork() child of the parent that outlives it inherits the write end of the sentinel pipe, so the workers exit only once that child is gone too (measured: 8 s later, then everything gone). Sibling workers and subprocess children do not hold it.

  3. Behaviour change worth a line in the description (measured). When the parent dies without shutdown, the resource_tracker now prints UserWarning: resource_tracker: There appear to be N leaked semaphore objects to clean up at shutdown on stderr and unlinks them. Before, the same semaphores leaked silently forever. An embedding host that calls gt4py.next.wait_for_compilation() at finalization sees no warning.

  4. Test portability nit (reasoned). _is_alive (tests/next_tests/unit_tests/otf_tests/test_runners.py:125) uses os.kill(pid, 0), which on Windows terminates the process; signal.SIGKILL and /proc do not exist there either. gt4py declares POSIX only and CI is Ubuntu, so a skipif(sys.platform == "win32") is enough to make the intent explicit.

  5. Test robustness nit (reasoned). parent.stdout.readline() (line 148) has no timeout and the project has no pytest-timeout, so a child that hangs before printing hangs the test instead of failing it. The Popen is never closed (ResourceWarning), and an early assertion failure leaves the sigkill arm's child sleeping out its 60 s. All bounded; I saw no flakiness.

  6. Comments. The helper's comment at lines 105-106 restates what the script does. The second paragraph of the _exit_with_parent docstring (the worker holds the call queue's write end, so it can never see EOF) is the non-obvious part and should stay.

Torn cache entries

No build system can hand out a half-written entry, with or without this change. Each writes its completion marker last, atomically, after the build subprocess has returned, and each reader requires it: gt4py.json status COMPILED via file_utils.atomic_write_bytes after check_call (build_systems/cmake.py:186, compiledb.py:242), read back by compiler.is_usable (compiler.py:29-37), which also treats a truncated json as no data; the compile-commands prototype is reused only if compile_commands.json parses, and it is written after build() (compiledb.py:296-311, 396-398); dace's .gt4py_compile_complete marker, without which the library is deleted before dace is asked to build (dace/workflow/compilation.py:332-347).

Measured with a persistent cache: killing the parent while cc1plus runs leaves worker and tracker gone within the first 50 ms scan, the orphaned compile finishes 0.8 s later and leaves a .o under status CONFIGURED, and the next run rebuilds and returns the right result. With the initializer from main under the same kill, the worker survived until killed by hand 415 s later, finished the compile, wrote COMPILED, and kept the parent's stdout pipe open.

Can two linkers, the orphaned one and the rerun's, interleave into one torn .so under a COMPILED marker? Ruled out on this toolchain: strace of the real link and compile commands shows GNU ld 2.46 and as both unlink the output and then openat(O_CREAT|O_TRUNC) a fresh inode, so each writer has its own file and the surviving directory entry has a single writer; and the outputs are byte-identical across independent builds (6 .so and 5 .o from separate builds, one sha256 each). The remaining way to a torn module under COMPILED is that single writer being killed mid-write after the rerun already wrote the marker, i.e. a second kill. Not checked for other linkers (lld writes to a temp file and renames; gold not measured).

Verified

  • Python 3.12.12 and 3.14.0: test_runners.py 20 passed on both; the new test 2.5-2.7 s per arm, green over 5 repeats, under -n 2 three times and the module under -n 4; workers exit within the first 0.5 s scan, so the 30 s bound has ample headroom, and a worker still importing gt4py when the parent dies also exits, because the initializer's join() returns at once on a closed sentinel.
  • Stdlib reading, 3.12 diffed against 3.14: spawn_main, _ParentProcess.join and the sentinel plumbing are identical. The sentinel's write end is closed only by the Popen finalizer, after the pool has joined the worker, or by parent death, so no new BrokenProcessPool path; max_tasks_per_child is not used; reset followed by a fresh pool is covered by the existing tests.
  • Windows (reasoned): the sentinel is the parent's process handle, wait() maps to WaitForMultipleObjects, os._exit works from any thread. macOS (reasoned): same POSIX code path.

Not verified

GPU builds, macOS, Windows, the full nox -s test_next session, and the embedded ICON/HPC path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants