From fc7706dfff1cd7de9bffb11921dc9e849d750f74 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 22 Oct 2026 02:20:47 +0200 Subject: [PATCH] fix(dev): give serve the full shutdown grace before killing it - dev.ShutdownGrace (internal, 15 seconds, longer than serve's 10-second drain) is the grace child.stop waits after SIGTERM, instead of 2 seconds - summer's delegates use the same constant (appShutdownGrace) - scaffolding docs say summer dev waits for serve's shutdown report (WR-03) --- cmd/summer/runtime.go | 8 ++++---- docs/console/scaffolding.md | 2 +- internal/dev/watch.go | 11 +++++++++- internal/dev/watch_test.go | 41 +++++++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/cmd/summer/runtime.go b/cmd/summer/runtime.go index 3026f204..f903e695 100644 --- a/cmd/summer/runtime.go +++ b/cmd/summer/runtime.go @@ -11,9 +11,9 @@ import ( "path/filepath" "strings" "syscall" - "time" "git.golem15.com/golem15/summercms/internal/build" + "git.golem15.com/golem15/summercms/internal/dev" "git.golem15.com/golem15/summercms/modules/bonfire" ) @@ -181,9 +181,9 @@ func runAppCmd(cmd *exec.Cmd, argv []string) error { } // appShutdownGrace is how long a cancelled app binary may take to exit after -// SIGTERM before it is killed. It is longer than the 10-second shutdown -// timeout of serve and the workers, so they finish a graceful shutdown. -const appShutdownGrace = 15 * time.Second +// SIGTERM before it is killed: the grace summer dev gives serve as well, +// longer than the 10-second shutdown timeout of serve and the workers. +const appShutdownGrace = dev.ShutdownGrace // newAppCmd builds the command that runs the app binary bin with argv in // dir, writing to stdout and stderr (see appStreams). The child inherits diff --git a/docs/console/scaffolding.md b/docs/console/scaffolding.md index 07506195..9e0de998 100644 --- a/docs/console/scaffolding.md +++ b/docs/console/scaffolding.md @@ -20,7 +20,7 @@ summer build summer dev ``` -`summer dev` passes its global options on to `serve`: `summer dev -v` shows the per-plugin lines of the startup summary and every component's info logs, `summer dev -q` only warnings and errors. Ctrl+C stops `serve`, which prints its shutdown report, then `summer dev` prints its own report (rebuilds, failed builds, restarts, uptime) and exits 0. +`summer dev` passes its global options on to `serve`: `summer dev -v` shows the per-plugin lines of the startup summary and every component's info logs, `summer dev -q` only warnings and errors. Ctrl+C stops `serve` and waits for its shutdown report: `serve` drains the requests in flight for up to 10 seconds, and `summer dev` kills it only if it is still running 15 seconds after the signal (a restart after a rebuild waits the same way). Then `summer dev` prints its own report (rebuilds, failed builds, restarts, uptime) and exits 0. Do not edit `main.go` or `plugins.gen.go`: `summer build` rewrites them from the manifest. diff --git a/internal/dev/watch.go b/internal/dev/watch.go index 794a8d4c..548c8d76 100644 --- a/internal/dev/watch.go +++ b/internal/dev/watch.go @@ -19,6 +19,13 @@ import ( const defaultDebounce = 200 * time.Millisecond +// ShutdownGrace is how long a stopped app binary may take to exit after +// SIGTERM before it is killed, for summer dev and summer's delegates alike. +// It is longer than the 10-second shutdown timeout of serve (its request +// drain) and the workers, so they finish a graceful shutdown and print +// their report. +const ShutdownGrace = 15 * time.Second + // Options customizes the watch loop. Tests inject Build and Start. type Options struct { Build func(ctx context.Context, appDir string, out io.Writer) error @@ -224,6 +231,8 @@ func startChild(ctx context.Context, opts Options, binPath string, args []string return &child{cmd: cmd, wait: ch}, nil } +// stop sends the child SIGTERM and waits for it to exit, killing it only +// after ShutdownGrace. func (c *child) stop() { if c == nil || c.cmd == nil || c.cmd.Process == nil { return @@ -231,7 +240,7 @@ func (c *child) stop() { _ = c.cmd.Process.Signal(syscall.SIGTERM) select { case <-c.wait: - case <-time.After(2 * time.Second): + case <-time.After(ShutdownGrace): _ = c.cmd.Process.Kill() <-c.wait } diff --git a/internal/dev/watch_test.go b/internal/dev/watch_test.go index 5d3a4abc..e36b8614 100644 --- a/internal/dev/watch_test.go +++ b/internal/dev/watch_test.go @@ -7,6 +7,7 @@ import ( "io" "os" "os/exec" + "os/signal" "path/filepath" "strings" "sync" @@ -620,3 +621,43 @@ func TestDevGracefulNil(t *testing.T) { t.Fatalf("no dev report:\n%s", out.String()) } } + +// TestStopHelperProcess is not a real test: TestDevStopWaitsForGracefulShutdown +// re-executes the test binary with DEV_STOP_HELPER=1 to get a child that, +// like serve draining requests, takes 2.5 seconds to shut down after +// SIGTERM and then exits 0. +func TestStopHelperProcess(t *testing.T) { + if os.Getenv("DEV_STOP_HELPER") != "1" { + return + } + sigs := make(chan os.Signal, 1) + signal.Notify(sigs, syscall.SIGTERM) + fmt.Println("ready") + select { + case <-sigs: + time.Sleep(2500 * time.Millisecond) + os.Exit(0) + case <-time.After(30 * time.Second): + os.Exit(3) + } +} + +// TestDevStopWaitsForGracefulShutdown (WR-03): child.stop sends SIGTERM +// and waits for a child that takes longer than two seconds to drain +// instead of killing it; the grace exceeds serve's 10-second drain. +func TestDevStopWaitsForGracefulShutdown(t *testing.T) { + if ShutdownGrace <= 10*time.Second { + t.Fatalf("ShutdownGrace %s does not exceed serve's 10-second drain", ShutdownGrace) + } + t.Setenv("DEV_STOP_HELPER", "1") + var out safeBuffer + c, err := startChild(t.Context(), Options{Start: startBin}, os.Args[0], []string{"-test.run=^TestStopHelperProcess$"}, &out, &out) + if err != nil { + t.Fatal(err) + } + waitUntil(t, "helper ready", func() bool { return strings.Contains(out.String(), "ready") }) + c.stop() + if st := c.cmd.ProcessState; st == nil || !st.Exited() || st.ExitCode() != 0 { + t.Fatalf("child was not allowed to finish its shutdown: %v", st) + } +}