#3801·gofr

Graceful stop establishes no happens-before edge: lost server shutdown (nil-check-as-started) + cron Stop() without job join

Author: akshat-kumar-singhalCreated Jul 28, 2026Updated Sep 2, 2026

Follow-up from the review on #3800. Two lifecycle bugs share one shape: a "stop" that establishes no happens-before edge with the thing it is stopping. #3800 fixes the memory-safety half (the -race-visible data race on srv); these two ordering bugs remain and are not visible to -race.

1. Lost shutdown: srv == nil doubles as "not started yet"

run.go starts the shutdown handler before the servers:

go
a.startShutdownHandler(ctx, timeout)   // parks on <-ctx.Done()
a.startTelemetryIfEnabled()
a.startAllServers(ctx)                 // servers assign srv in here

A termination signal arriving in that window is observed by a Shutdown() that sees srv == nil, reports success, and returns — then run() brings the listener up and serves forever:

 t0  SIGTERM arrives
 t1  shutdown g:  Shutdown() -> lock -> srv == nil -> unlock -> return nil  ("nothing to stop")
 t2  serve g:     s.srv = srv
 t3               srv.ListenAndServe()   <== serves forever
 t4  wg.Wait() never returns -> process hangs until SIGKILL

Reproduced on the #3800 branch: shutdown returns nil, then the listener comes up and stays up. In practice this is a SIGTERM landing right after start (crashloop, fast rollout, failed readiness) where the pod ignores graceful shutdown and hangs until SIGKILL.

Root cause: srv is both the server handle and the "has it started?" flag; a nil check carries no happens-before edge, so a mutex fixes the read/write hazard (#3800) but not the ordering. Applies to httpServer, metricServer, mcpServer.

Fix direction: a per-server started chan struct{} (or a lifecycle state) that Shutdown waits on — turning "not started yet" into "wait until started (or until start is known not to happen), then stop" instead of "nothing to do". Must also handle the case where the server errors out before assigning srv (cert validation failure, blocked port) so Shutdown cannot block forever.

2. Cron Stop() does not join in-flight jobs (and ignores the shutdown context)

go
func (c *Crontab) Stop() {
	c.once.Do(func() {
		c.ticker.Stop()
		close(c.done)
	})
}

Stop() halts future ticks but never joins a job that a tick already dispatched — runScheduled fires go j.run(container) with no WaitGroup. So a job dispatched just before Stop() keeps running after the caller has moved on. In tests this is the panic: Fail in goroutine after TestCronTab_AddJob has completed (a job hits the gomock mock after the test finished); #3800 mitigates it with defer c.Stop() + AnyTimes(), which narrows the window but cannot close it.

Separately, App.Shutdown(ctx) calls a.cron.Stop() (gofr.go) but Stop() takes no context, so it cannot honour the shutdown timeout. A naive wg.Wait() join would let a hung/long cron job block shutdown indefinitely — which is why this needs a deliberate, context-aware design rather than a drop-in.

Fix direction: add a WaitGroup around go j.run(...), give Stop(ctx context.Context) a context, and join in-flight jobs bounded by that context (wait, else ListenAndServe-style force-return on timeout). Wire the shutdown context through from App.Shutdown.

Why one issue

Both are the same defect shape; fixing them together keeps the "graceful stop must establish a happens-before edge with start/dispatch" invariant in one place rather than solving it twice. -race points at neither directly — it only flagged the adjacent memory hazard that #3800 closes.