Skip to content

Stop server on SIGTERM by default#406

Merged
Willenbrink merged 5 commits into
masterfrom
stop-sigterm
Mar 25, 2026
Merged

Stop server on SIGTERM by default#406
Willenbrink merged 5 commits into
masterfrom
stop-sigterm

Conversation

@yawaramin
Copy link
Copy Markdown
Member

Fix #174

@Willenbrink
Copy link
Copy Markdown
Collaborator

Looks good!

I was wondering if we should also handle SIGQUIT but that already works. See also this discussion for the common signals to terminate processes. SIGHUP might be interesting at some point.

@Willenbrink Willenbrink self-requested a review September 9, 2025 21:10
@aantron
Copy link
Copy Markdown
Collaborator

aantron commented Oct 3, 2025

if we should also handle SIGQUIT but that already works

In what context? I have SIGQUIT working in the shell to terminate Dream servers without this PR.

λ dune exec example/1-hello/hello.exe
03.10.25 07:11:33.858                       Running at http://localhost:8080
03.10.25 07:11:33.858                       Type Ctrl+C to stop
^\fish: Job 1, 'dune exec example/1-hello/hello…' terminated by signal SIGQUIT (Quit request from job control with core dump (^\))

But interestingly SIGTERM also also works:

λ dune exec example/1-hello/hello.exe
03.10.25 07:12:58.001                       Running at http://localhost:8080
03.10.25 07:12:58.001                       Type Ctrl+C to stop
^Zfish: Job 1, 'dune exec example/1-hello/hello…' has stopped

λ bg
Send job 1 'dune exec example/1-hello/hello.exe' to background

λ kill %1
λ fish: Job 1, 'dune exec example/1-hello/hello…' terminated by signal SIGTERM (Polite quit request)

This appears to be an issue inside containers. Is SIGQUIT relevant for Dream servers running inside containers?

Comment thread src/http/http.ml Outdated
@yawaramin
Copy link
Copy Markdown
Member Author

TODO: reconsider approach for multicore.

Don't install the signal handler at the module scope, to avoid polluting
the user's application with potentially unused handlers.
@Willenbrink
Copy link
Copy Markdown
Collaborator

It's a bit difficult to observe the effects because, when starting Dream from the terminal, it inherits the signal handlers from the shell. But the signal handling doesn't work on master but does on this branch when using e.g.

#!/bin/bash
trap '' SIGTERM SIGINT
setsid dune exec example/1-hello/hello.exe &
PID=$!

I've also added a SIGINT handler because AFAICT this is the default (see e.g. https://go-cookbook.com/snippets/context/graceful-shutdowns)

@Willenbrink Willenbrink merged commit 1fbb7fd into master Mar 25, 2026
6 of 7 checks passed
@Willenbrink Willenbrink deleted the stop-sigterm branch March 25, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

signal handling in dockerfile

3 participants