Skip to content

Shutdown is wrong in all three: Go blocks a fixed 30s without draining, Rust aborts in-flight requests, TS has no deadline #28

Description

@abienkowski

Summary

Shutdown is incorrect in all three implementations, in two different ways:

  • Go blocks for a fixed 30s on every signal and never actually waits for the drain. Graceful shutdown has never completed under Docker.
  • Rust exits promptly but aborts in-flight requests.
  • TypeScript drains correctly but has no deadline, so a request that never completes blocks exit forever.

Correction (2026-09-27): this issue originally described Go as the reference implementation that "already drains". That is wrong — see the Go section below. Go is the most broken of the three. Verified empirically on a real host, not by reading the code.

Affected implementation(s)

  • Go — blocks a fixed 30s, drain result never observed
  • Rust — no draining, in-flight requests aborted
  • TypeScript — drains, but unbounded: no deadline on the drain

Go: fixed 30s block, drain never awaited

main() (go/main.go:62-69 on main) waits on the timeout context itself:

shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 30*time.Second)
defer shutdownCancel()

<-shutdownCtx.Done()          // fires only when the 30s elapses
slog.Info("shutdown complete")

shutdownCtx is never cancelled early — defer shutdownCancel() runs after this line. So <-shutdownCtx.Done() is an unconditional 30-second sleep.

Meanwhile the actual server.Shutdown(shutdownCtx) runs in a detached goroutine (go/main.go:92-97 on main) whose completion nothing observes. The two are uncoordinated: main does not learn when draining finished, and draining does not shorten the wait.

Net effect: shutdown takes 30s whether there are in-flight requests or none at all.

Measured on a real host

Built binaries, native run against a live Docker daemon (not the container suites), SIGTERM with zero requests ever served:

Implementation Time from SIGTERM to exit
Go 30s
Rust 0s
TypeScript 0s

Go log, single run, no traffic:

20:17:48 INFO shutting down...
20:18:18 INFO shutdown complete     <- 30s later

Why this matters

docker stop sends SIGTERM and defaults to a 10s grace period before SIGKILL. The Go proxy needs 30s. It is therefore always SIGKILLed — graceful shutdown never once completes in a Docker deployment, and go is the default IMPL for deploy/.

Under systemd (TimeoutStopSec=90s default) it survives, but stalls every restart by 30s.

Suggested fix (Go)

Have serve() signal completion and have main wait on that, bounded by the deadline:

done := make(chan struct{})
go func() {
    <-ctx.Done()
    shutdownCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
    defer cancel()
    server.Shutdown(shutdownCtx)   // returns as soon as drain completes
    close(done)
}()
...
<-done                              // not <-shutdownCtx.Done()

Rust: no draining

Rust breaks its accept loops and exits main, dropping the tokio runtime and with it any in-flight serve_connection tasks, aborting responses mid-flight.

Suggested fix: track connections in a tokio::task::JoinSet (or counter + notify); on shutdown call hyper's Connection::graceful_shutdown, then await the set under tokio::time::timeout(Duration::from_secs(30), ...).

TypeScript: no deadline on the drain

Correction: an earlier revision of this issue listed TS alongside Rust as "drops in-flight requests". That is wrong. Node's server.close() does wait for active requests to finish. The real gaps are narrower.

shutdown() (ts/src/index.ts:118-127) calls server.close(cb) and nothing else:

function shutdown(signal: string) {
  console.log(`received ${signal}, shutting down...`);
  server.close(() => { console.log("server closed"); process.exit(0); });
}

Two gaps:

  1. Idle keep-alive connections delay exit. Retracted 2026-09-27 — measured, and false on the supported runtime. Node 19 changed server.close() to release idle keep-alive connections itself. Probed directly on Node 22 with a parked Agent({keepAlive:true}) socket: the close callback fires in 1ms with or without an explicit closeIdleConnections() call. No fix is needed and none was made; a regression test guards the behaviour in case the runtime is ever downgraded to 18.
  2. No deadline. A slow or stuck request delays exit indefinitely — there is no equivalent of Go's 30s cap. This is the only real TS defect.

Also re-entrant: a second SIGTERM calls server.close() again on an already-closing server.

Expected behavior (all three)

On shutdown signal:

  1. Stop accepting new connections.
  2. Allow in-flight requests to complete, up to a 30s deadline.
  3. Exit as soon as the drain completes, or at the deadline — whichever comes first.

Go gets point 3 wrong (fixed 30s regardless). Rust gets point 2 wrong (aborts instead of draining). TS gets point 2's deadline wrong (unbounded).

Resolution plan

No new dependencies required in any of the three.

Go

Have serve() report completion rather than main guessing at it:

// serve returns a channel closed once the server has finished draining.
func serve(ctx context.Context, listener net.Listener, addr string, handler http.Handler) <-chan struct{} {
	server := &http.Server{Handler: handler}
	done := make(chan struct{})

	go func() {
		defer close(done)
		<-ctx.Done()
		shutdownCtx, cancel := context.WithTimeout(context.Background(), shutdownTimeout)
		defer cancel()
		if err := server.Shutdown(shutdownCtx); err != nil {
			slog.Error("drain deadline exceeded", "error", err)
		}
	}()

	go func() {
		slog.Info("listening", "network", "unix", "addr", addr)
		if err := server.Serve(listener); err != nil && !errors.Is(err, http.ErrServerClosed) {
			slog.Error("server error", "addr", addr, "error", err)
		}
	}()
	return done
}

main becomes done := serve(...), then <-ctx.Done(), then <-done. This also collapses the 30s constant currently duplicated at main.go:81 and main.go:186 into a single shutdownTimeout.

Shutdown already closes idle keep-alive connections immediately and waits only on active ones, so idle exit drops to ~0s.

Rust

Breaking the accept loop is not sufficient — serve_connection tasks are detached via tokio::spawn, so dropping the runtime aborts them. Both of these are needed:

  1. Collect connections in a tokio::task::JoinSet instead of bare tokio::spawn, and after the loop breaks await it under tokio::time::timeout(SHUTDOWN_TIMEOUT, ...).
  2. Give each connection its own shutdown_tx.subscribe() so keep-alive connections are actively told to close:
let conn = http1::Builder::new().serve_connection(io, svc);
tokio::pin!(conn);
tokio::select! {
    res = conn.as_mut() => { /* finished naturally */ }
    _ = shutdown_rx.recv() => {
        conn.as_mut().graceful_shutdown();   // finish current request, then close
        let _ = conn.await;
    }
}

Step 1 alone would satisfy an idle-shutdown test but still hang on keep-alive traffic until the deadline.

Uses plain hyper, already a dependency. hyper-util has a GracefulShutdown helper but it requires enabling the server-graceful feature; avoided in keeping with the repo's minimal-dependency convention.

TypeScript

let shuttingDown = false;
function shutdown(signal: string) {
  if (shuttingDown) return;
  shuttingDown = true;
  console.log(`received ${signal}, shutting down...`);
  server.close(() => { console.log("server closed"); process.exit(0); });
  setTimeout(() => {
    console.error("drain deadline exceeded, forcing close");
    server.closeAllConnections();
    process.exit(1);
  }, SHUTDOWN_TIMEOUT_MS).unref();
}

closeAllConnections() is available on Node 18.2+; both the local toolchain and the StageX pallet-nodejs base are Node 22. closeIdleConnections() was in an earlier draft of this plan and was dropped: it is a no-op on Node >= 19 (see the retraction above).

In scope for this issue: extracting shutdown out of ts/src/index.ts so it can be unit tested. It is currently top-level module code with no export, so importing it runs the entire proxy — there is no way to write a regression test against it as-is. This is the only structural refactoring in the plan.

Test gap

The integration suites (27 tests x 3) pass regardless, because nothing asserts shutdown timing or in-flight completion. That is why this survived: the suites assert HTTP status codes only and cannot observe process lifecycle.

Each implementation should get a regression test asserting (a) an idle proxy exits well under the deadline, and (b) an in-flight request still completes:

Approach
Go Unit test on serve() — it already takes a net.Listener. Bind via the existing shortTempDir(t) helper (go/main_test.go:94), cancel the context, assert <-done closes in <1s.
Rust #[tokio::test] on spawn_unix_listener — same shape, already takes a listener and a shutdown_rx.
TS Requires the shutdown extraction above, then a node:test case against a real server on a temp Unix socket.

Note for the Go test: on macOS sun_path is capped at 104 bytes and t.TempDir() exceeds it, hence shortTempDir.

Context

Rust half split out from review follow-ups on PR #27. Go half found during real-machine verification of PR #38 — pre-existing on main, not introduced by that PR. Related: #25, #30.

Line references above are against main; they shift once #38 lands (the Go shutdown block moves to main.go:81-85 and serve() to main.go:182-195).

No activity

Activity on this issue will appear here.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Priority: P1Added to issues and PRs relating to a high severity bugs.Type: BugAdded to issues and PRs if they are addressing a bug

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions