Merge pull request #4531 from ATKasem/fix/postshutdown-hook-error-and-double-fire

🐛 fix: OnPostShutdown hook receives the real error and fires once

Author: ReneWerner87

app.go

@@ -1325,7 +1325,9 @@ func (app *App) ShutdownWithContext(ctx context.Context) error {
 
 	// Execute the Shutdown hook
 	app.hooks.executeOnPreShutdownHooks()
-	defer app.hooks.executeOnPostShutdownHooks(err)
+	// Use a closure so the hooks receive the final error; a plain
+	// `defer ...(err)` would capture the nil value at registration time.
+	defer func() { app.hooks.executeOnPostShutdownHooks(err) }()
 
 	err = app.server.ShutdownWithContext(ctx)
 	return err

listen.go

@@ -606,18 +606,13 @@ func (app *App) printRoutesMessage() {
 func (app *App) gracefulShutdown(ctx context.Context, cfg *ListenConfig) {
 	<-ctx.Done()
 
-	var err error
-
+	// The OnPostShutdown hooks are fired by ShutdownWithContext (via
+	// Shutdown/ShutdownWithTimeout) with the real error, so we must not fire
+	// them again here or they would run twice. That error is already delivered
+	// to those hooks, so it is intentionally ignored here.
 	if cfg != nil && cfg.ShutdownTimeout != 0 {
-		err = app.ShutdownWithTimeout(cfg.ShutdownTimeout) //nolint:contextcheck // TODO: Implement it
+		_ = app.ShutdownWithTimeout(cfg.ShutdownTimeout) //nolint:errcheck,contextcheck // error is delivered to OnPostShutdown hooks
 	} else {
-		err = app.Shutdown() //nolint:contextcheck // TODO: Implement it
+		_ = app.Shutdown() //nolint:errcheck,contextcheck // error is delivered to OnPostShutdown hooks
 	}
-
-	if err != nil {
-		app.hooks.executeOnPostShutdownHooks(err)
-		return
-	}
-
-	app.hooks.executeOnPostShutdownHooks(nil)
 }

listen_test.go

@@ -53,6 +53,97 @@ func Test_Listen_Graceful_Shutdown(t *testing.T) {
 	})
 }
 
+// go test -run Test_ShutdownWithContext_PostShutdownHookReceivesError
+func Test_ShutdownWithContext_PostShutdownHookReceivesError(t *testing.T) {
+	app := New()
+	app.Get("/", func(c Ctx) error {
+		time.Sleep(10 * time.Millisecond)
+		return c.SendString("ok")
+	})
+
+	hookErr := make(chan error, 1)
+	app.Hooks().OnPostShutdown(func(err error) error {
+		hookErr <- err
+		return nil
+	})
+
+	ln := fasthttputil.NewInmemoryListener()
+	go func() {
+		_ = app.Listener(ln, ListenConfig{DisableStartupMessage: true}) //nolint:errcheck // not needed
+	}()
+
+	require.Eventually(t, func() bool {
+		conn, err := ln.Dial()
+		if err == nil {
+			_ = conn.Close() //nolint:errcheck // not needed
+			return true
+		}
+		return false
+	}, time.Second, 20*time.Millisecond, "server failed to become ready")
+
+	// Keep a request in flight so shutdown cannot drain before the 1ns deadline.
+	conn, err := ln.Dial()
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = conn.Close() }) //nolint:errcheck // not needed
+	_, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: example.com\r\n"))
+	require.NoError(t, err)
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Nanosecond)
+	defer cancel()
+	shutdownErr := app.ShutdownWithContext(ctx)
+	require.Error(t, shutdownErr)
+
+	select {
+	case got := <-hookErr:
+		// The hook must receive the actual shutdown error, not the nil value
+		// captured when the defer was registered.
+		require.Equal(t, shutdownErr, got)
+	case <-time.After(time.Second):
+		t.Fatal("OnPostShutdown hook was not called")
+	}
+}
+
+// go test -run Test_GracefulShutdown_PostShutdownFiresOnce
+func Test_GracefulShutdown_PostShutdownFiresOnce(t *testing.T) {
+	app := New()
+	app.Get("/", func(c Ctx) error { return c.SendString("ok") })
+
+	fires := make(chan error, 8)
+	app.Hooks().OnPostShutdown(func(err error) error {
+		fires <- err
+		return nil
+	})
+
+	ln := fasthttputil.NewInmemoryListener()
+	gctx, gcancel := context.WithCancel(context.Background())
+	go func() {
+		_ = app.Listener(ln, ListenConfig{DisableStartupMessage: true, GracefulContext: gctx}) //nolint:errcheck,contextcheck // not needed
+	}()
+
+	require.Eventually(t, func() bool {
+		conn, err := ln.Dial()
+		if err == nil {
+			_ = conn.Close() //nolint:errcheck // not needed
+			return true
+		}
+		return false
+	}, time.Second, 20*time.Millisecond, "server failed to become ready")
+
+	gcancel() // trigger graceful shutdown
+
+	// The hook must fire exactly once.
+	select {
+	case <-fires:
+	case <-time.After(2 * time.Second):
+		t.Fatal("OnPostShutdown was not called")
+	}
+	select {
+	case <-fires:
+		t.Fatal("OnPostShutdown fired more than once")
+	case <-time.After(300 * time.Millisecond):
+	}
+}
+
 func testGracefulShutdown(t *testing.T, shutdownTimeout time.Duration) {
 	t.Helper()