Send SIGTERM before SIGKILL when shutting down child processes by robinbb · Pull Request #14170 · ocaml/dune

robinbb · 2026-04-13T03:17:14Z

Summary

Helps with #2445

When dune is interrupted (e.g., Ctrl-C) or a build is cancelled, child
processes now receive SIGTERM first, with a 200ms grace period for cleanup
handlers to run, before escalating to SIGKILL.

Previously, SIGKILL was sent immediately, preventing child processes from
running any cleanup logic (e.g., removing temp files, flushing logs).

Changes

kill_and_wait_for_all_processes: sends SIGTERM, spawns a background thread
that sends SIGKILL after 200ms
On Windows, where SIGTERM is not meaningful, behavior is unchanged
New cram test graceful-shutdown.t verifies that a child's SIGTERM handler
runs before the process is killed

rgrinberg · 2026-04-13T10:44:40Z

Thanks for working on this. This is rather annoying limitation indeed. I've pointed out why sending sigkill in a background thread isn't great and proposed a couple of workarounds.

rgrinberg

Did some minor tweaks to the test to make it a bit more reliable. I've disabled it entirely on macos for now.

I'll get rid of sigwait on macos. The issue is a bit ridiculous anyway.

I'll give you the honors of writing the CHANGES entry.

When dune is interrupted (SIGINT), it sends SIGKILL directly to child processes without first sending SIGTERM. This prevents cleanup handlers from running. The test currently documents the broken behavior. After the fix, the expected output should change from "cleanup did not run" to "cleanup ran". Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Fixes ocaml#2445 When dune shuts down (e.g., Ctrl-C), child processes now receive SIGTERM first, with a 200ms grace period for cleanup handlers to run, before escalating to SIGKILL. Previously, SIGKILL was sent immediately, preventing any cleanup. This changes the shutdown path (kill_and_wait_for_all_processes) only. The build cancellation path (on_cancel in watch mode) still uses immediate SIGKILL for snappy rebuilds. On Windows, where SIGTERM is not meaningful, the behavior is unchanged (SIGKILL is sent directly). Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Dune hangs on SIGINT on macOS due to a known sigwait issue (see signal_watcher.ml). The SIGTERM-before-SIGKILL logic is platform-independent; this test just cannot exercise it on macOS. Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

Signed-off-by: Robin Bate Boerop <me@robinbb.com>

robinbb · 2026-04-16T21:22:53Z

Did some minor tweaks to the test to make it a bit more reliable. I've disabled it entirely on macos for now.

This is great! Thank you. I've inverted the logic on the macos check to what I think you meant.

I'll get rid of sigwait on macos. The issue is a bit ridiculous anyway.

I'll give you the honors of writing the CHANGES entry.

Coming up...

Signed-off-by: Robin Bate Boerop <me@robinbb.com>

The SIGTERM-before-SIGKILL grace period in on_cancel caused stray-process.t to fail on macOS. Keep SIGTERM only in the shutdown path (kill_and_wait_for_all_processes), where cleanup matters. Signed-off-by: Robin Bate Boerop <me@robinbb.com>

robinbb · 2026-04-16T23:56:45Z

@rgrinberg I am concerned that merging now would be an abuse of your PR approval because the code has changed again since your approval. The concern: your approved version had SIGTERM in on_cancel via alarm clock, and I've now reverted that to immediate SIGKILL. Your approval message said "Did some minor tweaks to the test" — the tweaks assumed SIGTERM in on_cancel would work. But it caused stray-process.t to fail on macOS.

The current form of the PR still delivers the core value: SIGTERM before SIGKILL on shutdown (Ctrl-C exits). Watch-mode rebuilds use immediate SIGKILL as before. I think that this is the right trade-off because the issue (#2445) is about shutdown cleanup, not rebuild cancellation. Please merge if you agree.

rgrinberg · 2026-04-17T09:08:33Z

I think the issue is the same for watch mode and batch mode. We want to give actions the opportunity to clean themselves between build iterations, so that watch mode builds are as clean as possible.

The PR can of course be merged without doing this, but I don't think we can close the ticket, until we fix the issue for watch mode. Do you have an example of a failure you saw on macos?

rgrinberg · 2026-04-17T09:12:27Z

At the very least, you can disable the correct cleanup just on macos. I think that's acceptable as well.

robinbb · 2026-04-17T16:18:02Z

Do you have an example of a failure you saw on macos?

https://github.com/ocaml/dune/actions/runs/24534778323/job/71728235067

robinbb · 2026-04-17T16:18:54Z

The PR can of course be merged without doing this, but I don't think we can close the ticket, until we fix the issue for watch mode.

Yes, agreed. I will modify the description so that the PR merge does not close the issue, and I'll merge.

rgrinberg · 2026-04-17T17:50:58Z

I just saw this flake even without your change:

https://github.com/ocaml/dune/actions/runs/24575528376/job/71860957183?pr=14210

Signed-off-by: Robin Bate Boerop <me@robinbb.com>

## Summary - Remove `doc/changes/fixed/14170.md` which was included prematurely in #14170 Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Fixes #2445 - Extend the SIGTERM-before-SIGKILL grace period to the `on_cancel` path (watch mode rebuilds). Previously, only the shutdown path (`kill_and_wait_for_all_processes`) sent SIGTERM first (#14170). Now watch-mode cancellations also give child processes a chance to clean up before SIGKILL. - Uses the alarm clock to schedule SIGKILL after the grace period, and cancels the alarm if the process exits before the deadline, avoiding unnecessary delays to rebuilds. --------- Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Alizter · 2026-04-23T15:57:49Z

+    let deadline = Time.add (Time.now ()) sigterm_grace_period in
+    let sent_sigkill = ref false in
+    while Event.Queue.pending_jobs t.events > 0 do
+      ignore (Process_watcher.wait_unix t.process_watcher : Fiber.fill list);


This does not work on Windows unfortunately and causes an internal error when using Ctrl + C on Windows.

I'll create an issue

Fixes ocaml#14302. PR ocaml#14170 added a SIGTERM-before-SIGKILL grace period to [kill_and_wait_for_all_processes]. The new polling loop calls [Process_watcher.wait_unix] unconditionally, which on Windows routes through [Proc.wait] and raises: Code_error "wait4 not available on windows" Since SIGTERM is not meaningful on Windows anyway (as PR ocaml#14170 itself noted), skip the grace-period pre-phase there and send SIGKILL directly. The main drain loop already handles the Windows case correctly: job exits are pushed to [jobs_completed] by the [run_win32] polling thread and decrement [pending_jobs] when popped by [Event.Queue.next]. Signed-off-by: Robin Bate Boerop <me@robinbb.com>

## Summary Fixes #14302. PR #14170 added a SIGTERM-before-SIGKILL grace period to `kill_and_wait_for_all_processes`. The new polling loop calls `Process_watcher.wait_unix` unconditionally, which on Windows routes through `Proc.wait` and raises `Code_error "wait4 not available on windows"`, crashing dune on Ctrl+C. ## Fix Since SIGTERM is not meaningful on Windows anyway — as PR #14170 itself noted ("On Windows, where SIGTERM is not meaningful, behavior is unchanged") — skip the grace-period pre-phase on Windows and send SIGKILL directly. The existing main drain loop already handles the Windows case correctly: job exits are pushed to `jobs_completed` by the `run_win32` polling thread and decrement `pending_jobs` when popped by `Event.Queue.next` (see `src/dune_scheduler/event.ml:222` and `:187-191`). Signed-off-by: Robin Bate Boerop <me@robinbb.com>

@robinbb

CHANGES: ### Fixed - Auto-inject `"menhir" {>= "20180523"}` into generated opam files when the menhir extension is used. Dune's menhir rules rely on `--infer-write-query` and `--infer-read-reply`, which require at least this version; without the lower bound, builds fail with older menhir installations. (ocaml/dune#10707, @robinbb) - Fix `--display=quiet` not suppressing "Entering directory" and "Leaving directory" messages when using `--root`. These messages are now deferred until there is actual output, so silent builds produce no noise. (ocaml/dune#12974, fixes ocaml/dune#12854, @Alizter) - Fix an internal error when a module is shared between a `rocq.theory` and `rocq.extraction` stanza. The error now includes a hint pointing to the conflicting stanza. (ocaml/dune#13733, @Durbatuluk1701) - Fix cookies defined on `ppx_rewriter` being lost when that rewriter was used as a dependency of another `ppx_rewriter`. (ocaml/dune#13737, fixes ocaml/dune#3426, @Alizter) - Improve opam file generation for packages that set `(dir ..)`. Such packages will now have a test target that is limited to this directory. (ocaml/dune#13778, @rgrinberg) - Stop duplicating dune diagnostics to subscribers over RPC (ocaml/dune#13816, @rgrinberg) - Fix dependency cycle when using the `package` with a library conditionally enabled via `enabled_if`. (ocaml/dune#13833, @toots) - Fix underspecification of dependencies in the sandbox for modules with interfaces. (ocaml/dune#13842, @anmonteiro) - Honor sandbox settings specified inside `(:include ..)` expressions in the `deps` field (ocaml/dune#13898, @rgrinberg) - Fix `dune pkg lock` failing when a `pin` stanza contains a `file://` URL with a relative path outside the workspace (ocaml/dune#13915, fixes ocaml/dune#10254, @shunueda) - Use all stat attributes to detect patch back source tree changes (ocaml/dune#13986, @rgrinberg) - Use device and inode number for invalidating cached digests (ocaml/dune#13991, @rgrinberg) - Stop trying to set SO_REUSEADDR on unix sockets when setting up dune rpc (ocaml/dune#14056, @rgrinberg) - Fix autolocking to correctly detect changes to `(depends)` in watch mode (ocaml/dune#14066, fixes ocaml/dune#13234, @Alizter) - Fix incremental builds for libraries using `(wrapped (transition ...))`. The compat shim modules were never recompiled when the inner module's interface changed, causing "inconsistent assumptions" errors. (ocaml/dune#14090, fixes ocaml/dune#14089, @Alizter) - Fix Rocq configuration detection to use `rocq c --config` subcommand instead of `rocq --config` (ocaml/dune#14093, fixes ocaml/dune#13774, @Durbatuluk1701) - Bump dune rpc's request pending request limit 10 to 100 (ocaml/dune#14094, @rginberg) - Invalidate stale cached digests in all build commands (ocaml/dune#14126, @rgrinberg) - Fix `root_module` generating duplicate module definitions when a findlib sub-package shares a directory with its parent (e.g., `logs.lwt` alongside `logs`). (ocaml/dune#14135, fixes ocaml/dune#6148, @robinbb) - Fix `dune subst` prepending a duplicate `version:` field to opam files that already contain one, instead of replacing it in place. (ocaml/dune#14136, fixes ocaml/dune#878, @robinbb) - Fix `@ocaml-index` alias being generated for all contexts, causing build errors in multi-context workspaces where some libraries are disabled in non-default contexts. The alias is now only generated for the merlin context. (ocaml/dune#14137, fixes ocaml/dune#12007, @robinbb) - Remove the warning about the deprecation of the coq stanza for dune lang before 3.21 since it has been introduced in this version (ocaml/dune#14187, @bobot) - `$ dune init` now generates projects with correct .opam files (ocaml/dune#14192, @rgrinberg) - Send SIGTERM before SIGKILL when cancelling child processes in watch mode, giving cleanup handlers a chance to run before escalating. (ocaml/dune#14224, ocaml/dune#14170, fixes ocaml/dune#2445, @robinbb) - Correctly specify dependencies for `generate_runner` in inline tests (ocaml/dune#14276, @rgrinberg) - Fix duplicate dune version bounds in generated opam files. When users declare `(dune (>= X.Y))` matching or exceeding the `(lang dune X.Y)` version, the generated opam `depends` no longer contains redundant constraints like `{>= "2.7" & >= "2.7"}`. (ocaml/dune#3916, ocaml/dune#11106, @robinbb) - Fix inline tests not being executed for byte-only libraries. When a library has `(modes byte)`, the inline test runner is now linked in byte mode so the library's test code is included. (ocaml/dune#9757, @robinbb) ### Added - Add support for `c_library_flags` in foreign_stubs (ocaml/dune#13484, @madroach) - Move the management of Jsoo config details out of dune (ocaml/dune#13613, @vouillon) - Js_of_ocaml: share standalone runtimes (ocaml/dune#13621, @vouillon) - Allow multiple `(dirs ..)` stanzas in the same Dune file. Starting with `(lang dune 3.23)`, Dune takes the union of the specified directories (ocaml/dune#13734, fixes ocaml/dune#6249, @anmonteiro) - Sandboxed rules are now allowed to produce diff promotions when `(corrections produce)` is set on the rules. Whenever such an action produces `foo.corrected`, it will be automatically registered as a promotion for `foo` (ocaml/dune#13813, @rgrinberg) - `$ dune clean` now accepts arguments to only remove certain targets from the _build directory (ocaml/dune#13875, @rgrinberg) - `$ dune promote` now promotes all paths that are a prefix of the path provided. For example, `$ dune promote foo` will promote `foo`, `foo/bar`, `foo/bar/baz` (but not `foobar`). (ocaml/dune#13876, @rgrinberg) - Allow for the `diff` action to diff entire directories (ocaml/dune#13880, fixes ocaml/dune#3567, @rgrinberg) - A hint is now emitted when trying to build a target inside a directory that was excluded by a `(dirs ...)` stanza (ocaml/dune#13919, @mefyl). - Add --debug-backtraces to `$ dune {promote,trace,cache}` (ocaml/dune#13933, @rgrinberg) - Actions are now able to observe their project directory via `DUNE_PROJECT_ROOT` (ocaml/dune#13934, @rgrinberg) - Support hardlink sandboxing on Windows. (ocaml/dune#13987, addresses part of ocaml/dune#4362, @Alizter) - `rocq.extraction`: Add `extracted_files` field in `(rocq 0.13)`, replacing `extracted_modules`, supporting extraction to languages other than OCaml (ocaml/dune#13997, @Durbatuluk1701). - Infer source directories as dependencies when they're as reference directories in `diff` actions. - Add trace events for build start/stop/restarts (ocaml/dune#14163, ocaml/dune#14166, @rgrinberg) - Generate `compile_commands.json` for C/C++ foreign stubs when building `@check` with `(lang dune 3.23)`. This enables `clangd`, `ccls`, and other tools that consume compilation databases. (ocaml/dune#14185, fixes ocaml/dune#3531, @Alizter) - Add trace events for accepting clients and shutting down RPC (ocaml/dune#14232, @rgrinberg) - Back up the default trace file as `trace.csexp.old` before each build, preserving the previous trace for comparison. This does not apply when `--trace-file` is used. (ocaml/dune#14269, @Alizter) ### Changed - Dune digests target files and directories in background threads (ocaml/dune#13341, @rgrinbreg) - `Dyn.pp` now prints valid OCaml literals for more constructors: - `Char` is quoted (e.g., `'a'`) - `Int32`/`Int64`/`Nativeint` include literal suffixes (`l`/`L`/`n`) - `Float` handles `infinity`, `neg_infinity`, and `nan` (ocaml/dune#13394, grants ocaml/dune#13378, @Alizter) - Bump the minimal OCaml version required to build Dune from 4.08 to 4.14. (ocaml/dune#13533, @Alizter) - Starting from 3.23, user rules are sandboxed by default (ocaml/dune#13805, @rgrinberg) - Dune will no longer stat paths in the _build directory. This was done as protection from user rules accidentally touching things in the _build directory. This is forbidden, and dune offers rule sandboxing to prevent this (ocaml/dune#13875, @rgrinberg) - Improve mtime precision by no longer approximating them with floats (ocaml/dune#13962, @rgrinberg). - Remove `--makefile` (or `-m`) in `$ dune rules`. The sexp output is the only supported way of reflecting dune rules (ocaml/dune#14069, @rgrinberg) - Do not automatically promote files. Now, opam files must be manually promoted with `$ dune promote`. Their generation is triggered by `@install`, `@runtest`, and `@opam`. In release mode, .opam files aren't generated at all and whatever is in the source is used (ocaml/dune#14108, @rgrinberg) - Inline test runner generation is now sandboxed (ocaml/dune#14257, @rgrinberg)

robinbb force-pushed the sigterm-before-sigkill branch 5 times, most recently from a86535e to f1f3e3b Compare April 13, 2026 06:15

Leonidas-from-XIV requested a review from rgrinberg April 13, 2026 10:41

rgrinberg reviewed Apr 13, 2026

View reviewed changes

Comment thread src/dune_scheduler/scheduler.ml Outdated

robinbb force-pushed the sigterm-before-sigkill branch 6 times, most recently from f62cea7 to 883a32d Compare April 14, 2026 03:31

rgrinberg requested changes Apr 14, 2026

View reviewed changes

Comment thread src/dune_scheduler/scheduler.ml Outdated

Comment thread src/dune_scheduler/scheduler.ml

Comment thread src/dune_scheduler/scheduler.ml Outdated

robinbb force-pushed the sigterm-before-sigkill branch 5 times, most recently from 89e65d1 to 8cd5f12 Compare April 14, 2026 23:18

robinbb assigned rgrinberg Apr 15, 2026

robinbb marked this pull request as ready for review April 15, 2026 02:00

rgrinberg approved these changes Apr 15, 2026

View reviewed changes

robinbb and others added 5 commits April 16, 2026 14:17

test: skip graceful-shutdown test on macOS

609c69c

Dune hangs on SIGINT on macOS due to a known sigwait issue (see signal_watcher.ml). The SIGTERM-before-SIGKILL logic is platform-independent; this test just cannot exercise it on macOS. Signed-off-by: Robin Bate Boerop <me@robinbb.com>

cleanup test + some minor style

e442f08

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

fix: invert enabled_if to actually skip graceful-shutdown test on macOS

d0df9a3

Signed-off-by: Robin Bate Boerop <me@robinbb.com>

robinbb force-pushed the sigterm-before-sigkill branch from dc8b471 to d0df9a3 Compare April 16, 2026 21:20

robinbb added 2 commits April 16, 2026 14:23

Add changelog entry for SIGTERM before SIGKILL fix

99f5558

Signed-off-by: Robin Bate Boerop <me@robinbb.com>

robinbb merged commit 96d881c into ocaml:main Apr 17, 2026
30 checks passed

robinbb deleted the sigterm-before-sigkill branch April 17, 2026 16:20

robinbb added a commit that referenced this pull request Apr 17, 2026

Remove premature changelog entry for #14170

1ca9675

Signed-off-by: Robin Bate Boerop <me@robinbb.com>

This was referenced Apr 17, 2026

Remove premature changelog entry for #14170 #14215

Merged

Send SIGTERM before SIGKILL on watch-mode cancellation #14224

Merged

robinbb added a commit that referenced this pull request Apr 17, 2026

Remove premature changelog entry for #14170 (#14215)

271bf3c

## Summary - Remove `doc/changes/fixed/14170.md` which was included prematurely in #14170 Signed-off-by: Robin Bate Boerop <me@robinbb.com>

Alizter reviewed Apr 23, 2026

View reviewed changes

This was referenced Apr 23, 2026

3.23.0 release tracker #14211

Closed

Ctrl + C to terminate Dune on Windows no longer works #14302

Closed

Copilot AI mentioned this pull request Apr 23, 2026

Fix Ctrl+C crash on Windows caused by unguarded wait_unix call #14303

Closed

robinbb mentioned this pull request Apr 24, 2026

fix: guard SIGTERM grace period in shutdown on Windows (#14302) #14316

Merged

This was referenced Apr 30, 2026

[new release] dune (18 packages) (3.23.0~alpha0) ocaml/opam-repository#29822

Closed

[new release] dune (18 packages) (3.23.0~alpha2) ocaml/opam-repository#29824

Closed

[new release] dune (18 packages) (3.23.0) ocaml/opam-repository#29844

Merged

Alizter added this to the 3.23.0 milestone Jun 10, 2026

Uh oh!

Conversation

robinbb commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Uh oh!

Uh oh!

rgrinberg commented Apr 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rgrinberg left a comment

Choose a reason for hiding this comment

Uh oh!

robinbb commented Apr 16, 2026

Uh oh!

robinbb commented Apr 16, 2026

Uh oh!

rgrinberg commented Apr 17, 2026

Uh oh!

rgrinberg commented Apr 17, 2026

Uh oh!

robinbb commented Apr 17, 2026

Uh oh!

robinbb commented Apr 17, 2026

Uh oh!

Uh oh!

rgrinberg commented Apr 17, 2026

Uh oh!

Alizter Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Alizter Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Alizter Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

robinbb commented Apr 13, 2026 •

edited

Loading