zip: OnShutdown — LIFO lifecycle hooks for express-style composition (#4)

Add the one teardown primitive zip was missing: func (a *App) OnShutdown(fn func(context.Context) error) Subsystems register their own cleanup at mount time; reverse-mount teardown falls out for free. This is what consumers (hanzoai/cloud) need to flatten a MountSpec{Shutdown} registry into plain pkg.Mount(app, cfg) calls. Semantics: - Order: hooks run LAST — after listeners stop accepting and after in-flight requests drain (fiber) — in LIFO order. Draining first means a subsystem's teardown never races the requests still using it; LIFO gives reverse-mount teardown of dependencies. - Errors: every hook runs even if an earlier one fails; all hook errors (plus the drain error) are aggregated via errors.Join. - Context: ShutdownWithContext passes its ctx (values + deadline) to hooks; plain Shutdown uses context.Background() (process ending). - Idempotent: a shared once-guard makes the whole Shutdown a no-op on the second call; hooks run at most once, across both entry points. - Post-shutdown registration is a no-op: the hook is dropped and a warning logged (no panic). Keeps OnShutdown's meaning constant — register-for-later — instead of silently changing to run-now. - Registration is mutex-guarded; a nil hook is ignored. Decomplect: fold the internal `closers` stopgap onto OnShutdown. module.go's own comment described appendCloser as a poor-man's graceful-shutdown hook list "because fiber doesn't expose one" — which is exactly this primitive. Module() now registers mod.Close via OnShutdown, so there is ONE teardown mechanism. A module's Close now runs AFTER in-flight requests drain (it previously ran before), so a module stays live for any request still executing it — a correctness improvement. closers.go is deleted. Tests (shutdown_test.go): LIFO order, errors.Join aggregation, idempotent second Shutdown, ShutdownWithContext ctx propagation, post-shutdown drop, nil-hook, and concurrent registration under -race. Co-authored-by: Hanzo Dev <[email protected]>
2026-10-19 15:38:57 +00:00 · 2026-07-10 16:13:22 -07:00
parent ee9f707822
commit 509a645e32
5 changed files with 276 additions and 42 deletions
@@ -1,21 +0,0 @@
-package zip
-
-import "context"
-
-// appendCloser registers fn to run during Shutdown(). Used by Module()
-// to release extension-runtime modules.
-func (a *App) appendCloser(fn func() error) {
-	a.closers = append(a.closers, fn)
-}
-
-// runClosers runs all registered closers and returns the first error.
-func (a *App) runClosers(_ context.Context) error {
-	var firstErr error
-	for _, fn := range a.closers {
-		if err := fn(); err != nil && firstErr == nil {
-			firstErr = err
-		}
-	}
-	a.closers = nil
-	return firstErr
-}
@@ -1,6 +1,7 @@
 package zip

 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"path/filepath"
@@ -127,10 +128,10 @@ func (a *App) ModuleFn(method, path, fn, runtimeName, modulePath string) error {
 	}
 	a.method(method, path, handler)

-	// Register a shutdown hook so the module's resources are released.
-	// fiber doesn't expose a graceful-shutdown hook list, so we stash the
-	// closer on the App and run them inline from Shutdown if needed.
-	a.appendCloser(mod.Close)
+	// Release the module's resources on shutdown. Runs as a teardown hook,
+	// i.e. AFTER in-flight requests drain, so a module stays live for any
+	// request still executing it.
+	a.OnShutdown(func(context.Context) error { return mod.Close() })
 	return nil
 }

@@ -0,0 +1,72 @@
+package zip
+
+import (
+	"context"
+	"errors"
+)
+
+// OnShutdown registers fn as a teardown hook, run during Shutdown /
+// ShutdownWithContext. This is the one teardown primitive zip exposes:
+// subsystems register their own cleanup at mount time, and reverse-mount
+// teardown falls out for free (see the ordering note below).
+//
+// Ordering. Hooks run LAST in the shutdown sequence — after listeners stop
+// accepting and after in-flight requests drain — and in LIFO order (reverse
+// registration = reverse mount order). Draining first means a subsystem's
+// teardown never races the requests still using it; LIFO means a dependency
+// mounted before its dependents is torn down after them.
+//
+// Errors. Every hook runs even if an earlier one fails; all hook errors (and
+// the drain error) are aggregated with errors.Join and returned from Shutdown.
+//
+// Concurrency. Registration is safe from multiple goroutines. A nil fn is
+// ignored. Registering after Shutdown has begun is a no-op: the hook is
+// dropped (never run) and a warning is logged — there is no longer a shutdown
+// to hook into, and running it immediately would give OnShutdown two meanings
+// depending on timing. Register teardown at mount time, before Shutdown.
+func (a *App) OnShutdown(fn func(context.Context) error) {
+	if fn == nil {
+		return
+	}
+	a.hookMu.Lock()
+	if a.shuttingDown {
+		a.hookMu.Unlock()
+		a.logger.Warn("zip: OnShutdown called after Shutdown; hook dropped")
+		return
+	}
+	a.hooks = append(a.hooks, fn)
+	a.hookMu.Unlock()
+}
+
+// shutdown is the single graceful-shutdown sequence behind Shutdown and
+// ShutdownWithContext. It is idempotent: the first call claims shutdown under
+// hookMu (snapshotting and clearing the hooks) and every later call returns
+// nil without repeating any step, so hooks run at most once.
+//
+// Sequence: (1) stop every transport accepting new connections; (2) drain
+// in-flight requests via fiber, bounded by ctx; (3) run teardown hooks LIFO,
+// passing ctx to each. All errors are joined and returned.
+func (a *App) shutdown(ctx context.Context) error {
+	a.hookMu.Lock()
+	if a.shuttingDown {
+		a.hookMu.Unlock()
+		return nil
+	}
+	a.shuttingDown = true
+	hooks := a.hooks
+	a.hooks = nil
+	a.hookMu.Unlock()
+
+	// 1. Stop accepting new connections on every transport listener.
+	a.closeServers()
+	// 2. Drain in-flight requests (graceful; bounded by ctx).
+	errs := []error{a.fiber.ShutdownWithContext(ctx)}
+	// 3. Tear subsystems down in reverse mount order (LIFO). Run ALL hooks
+	//    even if some fail; aggregate every error.
+	for i := len(hooks) - 1; i >= 0; i-- {
+		if err := hooks[i](ctx); err != nil {
+			errs = append(errs, err)
+		}
+	}
+	return errors.Join(errs...)
+}
@@ -0,0 +1,174 @@
+package zip_test
+
+import (
+	"context"
+	"errors"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/zap-proto/zip"
+)
+
+// TestOnShutdown_LIFO proves hooks drain in reverse registration order
+// (reverse mount order) — the natural teardown of dependencies. Hooks run
+// sequentially in Shutdown's goroutine, so a plain slice needs no lock.
+func TestOnShutdown_LIFO(t *testing.T) {
+	app := zip.New(zip.Config{DisableStartupMessage: true})
+
+	var order []int
+	for i := 1; i <= 3; i++ {
+		i := i
+		app.OnShutdown(func(context.Context) error {
+			order = append(order, i)
+			return nil
+		})
+	}
+
+	if err := app.Shutdown(); err != nil {
+		t.Fatalf("Shutdown: %v", err)
+	}
+
+	want := []int{3, 2, 1}
+	if len(order) != len(want) {
+		t.Fatalf("order=%v, want %v", order, want)
+	}
+	for i := range want {
+		if order[i] != want[i] {
+			t.Fatalf("order=%v, want %v (LIFO teardown)", order, want)
+		}
+	}
+}
+
+// TestOnShutdown_JoinErrors proves every hook runs even when an earlier one
+// fails, and that all failures surface via errors.Join.
+func TestOnShutdown_JoinErrors(t *testing.T) {
+	app := zip.New(zip.Config{DisableStartupMessage: true})
+
+	errBoom := errors.New("boom")
+	errBang := errors.New("bang")
+	var ran int
+	app.OnShutdown(func(context.Context) error { ran++; return errBoom })
+	app.OnShutdown(func(context.Context) error { ran++; return nil })
+	app.OnShutdown(func(context.Context) error { ran++; return errBang })
+
+	err := app.Shutdown()
+	if ran != 3 {
+		t.Fatalf("ran=%d, want 3 (a failing hook must not stop the others)", ran)
+	}
+	if !errors.Is(err, errBoom) || !errors.Is(err, errBang) {
+		t.Fatalf("err=%v, want both boom and bang joined", err)
+	}
+}
+
+// TestShutdown_Idempotent proves the once-guard: a second call — even through
+// the other entry point — does not re-run hooks and returns nil.
+func TestShutdown_Idempotent(t *testing.T) {
+	app := zip.New(zip.Config{DisableStartupMessage: true})
+
+	var ran int
+	app.OnShutdown(func(context.Context) error { ran++; return nil })
+
+	if err := app.Shutdown(); err != nil {
+		t.Fatalf("Shutdown #1: %v", err)
+	}
+	if err := app.ShutdownWithContext(context.Background()); err != nil {
+		t.Fatalf("Shutdown #2: %v", err)
+	}
+	if ran != 1 {
+		t.Fatalf("ran=%d, want 1 (hooks run once across both entry points)", ran)
+	}
+}
+
+// TestShutdownWithContext_PropagatesCtx proves ShutdownWithContext passes its
+// context — both values and deadline — through to every hook.
+func TestShutdownWithContext_PropagatesCtx(t *testing.T) {
+	app := zip.New(zip.Config{DisableStartupMessage: true})
+
+	type ctxKey string
+	const key ctxKey = "k"
+	deadline := time.Now().Add(30 * time.Second)
+	ctx, cancel := context.WithDeadline(
+		context.WithValue(context.Background(), key, "v"), deadline)
+	defer cancel()
+
+	var gotVal any
+	var gotDeadline time.Time
+	var gotOK bool
+	app.OnShutdown(func(hctx context.Context) error {
+		gotVal = hctx.Value(key)
+		gotDeadline, gotOK = hctx.Deadline()
+		return nil
+	})
+
+	if err := app.ShutdownWithContext(ctx); err != nil {
+		t.Fatalf("ShutdownWithContext: %v", err)
+	}
+	if gotVal != "v" {
+		t.Fatalf("hook ctx value=%v, want %q", gotVal, "v")
+	}
+	if !gotOK || !gotDeadline.Equal(deadline) {
+		t.Fatalf("hook ctx deadline=%v ok=%v, want %v", gotDeadline, gotOK, deadline)
+	}
+}
+
+// TestOnShutdown_AfterShutdownDropped proves registering after Shutdown has
+// begun is a no-op: the hook never runs and nothing panics.
+func TestOnShutdown_AfterShutdownDropped(t *testing.T) {
+	app := zip.New(zip.Config{DisableStartupMessage: true})
+
+	if err := app.Shutdown(); err != nil {
+		t.Fatalf("Shutdown: %v", err)
+	}
+
+	var ran bool
+	app.OnShutdown(func(context.Context) error { ran = true; return nil }) // must not panic
+
+	// A subsequent Shutdown must not resurrect the late hook either.
+	if err := app.Shutdown(); err != nil {
+		t.Fatalf("Shutdown #2: %v", err)
+	}
+	if ran {
+		t.Fatal("late-registered hook ran; want dropped")
+	}
+}
+
+// TestOnShutdown_NilIgnored proves a nil hook is ignored at registration and
+// never panics the drain loop.
+func TestOnShutdown_NilIgnored(t *testing.T) {
+	app := zip.New(zip.Config{DisableStartupMessage: true})
+	app.OnShutdown(nil)
+	if err := app.Shutdown(); err != nil {
+		t.Fatalf("Shutdown: %v", err)
+	}
+}
+
+// TestOnShutdown_ConcurrentRegistration proves registration is safe under
+// concurrency (meaningful under -race): every hook registered before Shutdown
+// runs exactly once.
+func TestOnShutdown_ConcurrentRegistration(t *testing.T) {
+	app := zip.New(zip.Config{DisableStartupMessage: true})
+
+	const n = 50
+	var count int64
+	var wg sync.WaitGroup
+	wg.Add(n)
+	for i := 0; i < n; i++ {
+		go func() {
+			defer wg.Done()
+			app.OnShutdown(func(context.Context) error {
+				atomic.AddInt64(&count, 1)
+				return nil
+			})
+		}()
+	}
+	wg.Wait()
+
+	if err := app.Shutdown(); err != nil {
+		t.Fatalf("Shutdown: %v", err)
+	}
+	if count != n {
+		t.Fatalf("count=%d, want %d (all concurrently-registered hooks run)", count, n)
+	}
+}
@@ -27,8 +27,8 @@ import (
 	"errors"
 	"sync"

-	"github.com/zap-proto/fiber/v3"
 	luxlog "github.com/luxfi/log"
+	"github.com/zap-proto/fiber/v3"

 	"github.com/zap-proto/zip/internal/jsonenc"
 	"github.com/zap-proto/zip/runtime"
@@ -113,14 +113,21 @@ type Config struct {
 // App is the zip application. It wraps *fiber.App and exposes the zip
 // handler signature alongside generic typed handlers.
 type App struct {
-	cfg         Config
-	logger      luxlog.Logger
-	loader      runtime.Loader
-	fiber       *fiber.App
-	ops         []*registeredOp
-	closers     []func() error
-	servers     []Server // the running transport listeners, set by Listen
-	srvMu       sync.Mutex
+	cfg     Config
+	logger  luxlog.Logger
+	loader  runtime.Loader
+	fiber   *fiber.App
+	ops     []*registeredOp
+	servers []Server // the running transport listeners, set by Listen
+	srvMu   sync.Mutex
+
+	// Teardown lifecycle. hooks are drained LIFO by Shutdown after
+	// in-flight requests finish; shuttingDown guards against re-running
+	// them and against post-shutdown registration. hookMu guards both.
+	hooks        []func(context.Context) error
+	shuttingDown bool
+	hookMu       sync.Mutex
+
 	prepareOnce sync.Once // installs deferred routes (OpenAPI, MCP) exactly once
 }

@@ -180,18 +187,19 @@ func (a *App) Fiber() *fiber.App { return a.fiber }
 // Logger returns the App's logger.
 func (a *App) Logger() luxlog.Logger { return a.logger }

-// Shutdown gracefully stops both transports.
+// Shutdown gracefully stops every transport, then runs teardown hooks.
+// The process is ending, so hooks receive context.Background() — no
+// cancellation or deadline. Use ShutdownWithContext to bound teardown.
+// Idempotent: a second call is a no-op and hooks run at most once.
 func (a *App) Shutdown() error {
-	a.closeServers()
-	_ = a.runClosers(context.Background())
-	return a.fiber.Shutdown()
+	return a.shutdown(context.Background())
 }

-// ShutdownWithContext gracefully stops both transports bounded by ctx.
+// ShutdownWithContext is Shutdown bounded by ctx: ctx bounds the in-flight
+// drain and is passed to every teardown hook (values and deadline).
+// Shares Shutdown's once-guard, so mixing the two still runs hooks once.
 func (a *App) ShutdownWithContext(ctx context.Context) error {
-	a.closeServers()
-	_ = a.runClosers(ctx)
-	return a.fiber.ShutdownWithContext(ctx)
+	return a.shutdown(ctx)
 }

 // Use registers zip-style middleware. Each Handler runs in order; calling