fix: hand off PHP requests during configuration reloads

caddy/app.go

@@ -23,6 +23,7 @@ import (
 var (
 	options   []frankenphp.Option
 	optionsMU sync.RWMutex
+	activeApp atomic.Pointer[FrankenPHPApp]
 )
 
 // EXPERIMENTAL: RegisterWorkers provides a way for extensions to register frankenphp.Workers
@@ -140,6 +141,9 @@ func (f *FrankenPHPApp) collectOptions(repl *caddy.Replacer, keep bool) ([]frank
 		frankenphp.WithMaxIdleTime(f.MaxIdleTime),
 		frankenphp.WithMaxRequests(f.MaxRequests),
 	)
+	if f.httpApp != nil {
+		opts = append(opts, frankenphp.WithWorkerRequestDrainTimeout(time.Duration(f.httpApp.GracePeriod)))
+	}
 
 	usedWorkerNames := make(map[string]bool, len(f.Workers))
 
@@ -182,8 +186,10 @@ func (f *FrankenPHPApp) Start() error {
 
 	// if FrankenPHP is currently running, shut it down first
 	// this will happen in admin API reloads and caddy tests
+	activeApp.Store(f)
 	frankenphp.Shutdown()
 	if err := frankenphp.Init(f.opts...); err != nil {
+		activeApp.CompareAndSwap(f, nil)
 		return err
 	}
 
@@ -193,6 +199,9 @@ func (f *FrankenPHPApp) Start() error {
 }
 
 func (f *FrankenPHPApp) Stop() error {
+	f.hasStarted.Store(false)
+	activeApp.CompareAndSwap(f, nil)
+
 	if f.logger.Enabled(f.ctx, slog.LevelInfo) {
 		f.logger.LogAttrs(f.ctx, slog.LevelInfo, "FrankenPHP stopped 🐘")
 	}
@@ -215,41 +224,47 @@ func (f *FrankenPHPApp) Stop() error {
 // register workers and servers for "php" and "php_server" modules
 func (f *FrankenPHPApp) collectModuleOptions(repl *caddy.Replacer, usedWorkerNames map[string]bool, keep bool) ([]frankenphp.Option, error) {
 	opts := make([]frankenphp.Option, 0, len(f.modules))
-	serversByIndex := make(map[int]*frankenphp.Server, len(f.modules))
+	type registeredServer struct {
+		server *frankenphp.Server
+		name   string
+	}
+	serversByIndex := make(map[int]registeredServer, len(f.modules))
 
 	for _, module := range f.modules {
 		// modules with the same server_idx should share the same server instance
 		// example: the worker { match * } rule adds 2 "php" subroutes to the caddy handler
 		// the 2 handlers belong to the same "php_server" and must therefore share workers
 		if module.ServerIndex != 0 {
-			if server, ok := serversByIndex[module.ServerIndex]; ok {
+			if registered, ok := serversByIndex[module.ServerIndex]; ok {
 				if keep {
-					module.server = server
+					module.server = registered.server
+					module.reloadName = registered.name
 				}
 
 				continue
 			}
 		}
 
-		server, moduleOpts, err := f.collectModule(repl, module, usedWorkerNames)
+		serverName := f.resolveServerName(module)
+		server, moduleOpts, err := f.collectModule(repl, module, usedWorkerNames, serverName)
 		if err != nil {
 			return nil, err
 		}
 
 		if keep {
 			module.server = server
+			module.reloadName = serverName
 		}
 		if module.ServerIndex != 0 {
-			serversByIndex[module.ServerIndex] = server
+			serversByIndex[module.ServerIndex] = registeredServer{server: server, name: serverName}
 		}
 		opts = append(opts, moduleOpts...)
 	}
 
 	return opts, nil
 }
 
-func (f *FrankenPHPApp) collectModule(repl *caddy.Replacer, module *FrankenPHPModule, usedWorkerNames map[string]bool) (*frankenphp.Server, []frankenphp.Option, error) {
-	serverName := f.resolveServerName(module)
+func (f *FrankenPHPApp) collectModule(repl *caddy.Replacer, module *FrankenPHPModule, usedWorkerNames map[string]bool, serverName string) (*frankenphp.Server, []frankenphp.Option, error) {
 	server, err := frankenphp.NewServer(
 		module.resolvedDocumentRoot,
 		frankenphp.WithServerName(serverName),

caddy/module.go

@@ -59,6 +59,7 @@ type FrankenPHPModule struct {
 	requestEnv           frankenphp.PreparedEnv
 	requestOptions       []frankenphp.RequestOption
 	server               *frankenphp.Server
+	reloadName           string
 	logger               *slog.Logger
 	app                  *FrankenPHPApp
 }
@@ -187,11 +188,8 @@ func needReplacement(s string) bool {
 // ServeHTTP implements caddyhttp.MiddlewareHandler.
 func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ caddyhttp.Handler) error {
 	if !f.app.hasStarted.Load() {
-		// stall any incoming request if FrankenPHP has not started yet, blocking for up to 10 seconds
-		select {
-		case <-f.app.started:
-		case <-time.After(10 * time.Second):
-			return caddyhttp.Error(http.StatusServiceUnavailable, frankenphp.ErrNotRunning)
+		if err := f.app.waitForStartup(r.Context()); err != nil {
+			return err
 		}
 	}
 
@@ -226,9 +224,17 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 	}
 
 	err := f.server.ServeHTTP(w, r, opts...)
+	if err != nil {
+		if errors.Is(err, frankenphp.ErrNotRunning) {
+			if app := activeApp.Load(); app != nil && app != f.app {
+				return f.requestReload(w, r)
+			}
+			return caddyhttp.Error(http.StatusServiceUnavailable, err)
+		}
 
-	if _, rejected := errors.AsType[frankenphp.ErrRejected](err); err != nil && !rejected {
-		return caddyhttp.Error(http.StatusInternalServerError, err)
+		if _, rejected := errors.AsType[frankenphp.ErrRejected](err); !rejected {
+			return caddyhttp.Error(http.StatusInternalServerError, err)
+		}
 	}
 
 	return nil

caddy/reload.go

@@ -0,0 +1,83 @@
+package caddy
+
+import (
+	"context"
+	"net/http"
+	"time"
+
+	"github.com/caddyserver/caddy/v2/modules/caddyhttp"
+	"github.com/dunglas/frankenphp"
+)
+
+// requestReload dispatches r on the new runtime. Safe because the request
+// never started executing: its body is unread and no response was written.
+func (f *FrankenPHPModule) requestReload(w http.ResponseWriter, r *http.Request) error {
+	app := activeApp.Load()
+	if app == nil {
+		return caddyhttp.Error(http.StatusServiceUnavailable, frankenphp.ErrNotRunning)
+	}
+	if err := app.waitForStartup(r.Context()); err != nil {
+		return err
+	}
+	if !app.hasStarted.Load() || f.server == nil {
+		return caddyhttp.Error(http.StatusServiceUnavailable, frankenphp.ErrNotRunning)
+	}
+
+	// Runtime-generated server_N names depend on registration order across reloads.
+	name := f.reloadName
+	if old := f.app.reloadModule(name); old == nil || old.server != f.server {
+		return caddyhttp.Error(http.StatusServiceUnavailable, frankenphp.ErrNotRunning)
+	}
+	if m := app.reloadModule(name); m != nil {
+		return m.ServeHTTP(w, r, nil)
+	}
+	return caddyhttp.Error(http.StatusServiceUnavailable, frankenphp.ErrNotRunning)
+}
+
+func (f *FrankenPHPApp) waitForStartup(ctx context.Context) error {
+	select {
+	case <-f.started:
+		return nil
+	default:
+	}
+
+	wait := f.MaxWaitTime
+	if f.httpApp != nil {
+		if grace := time.Duration(f.httpApp.GracePeriod); grace > 0 && (wait == 0 || grace < wait) {
+			wait = grace
+		}
+	}
+	var expired <-chan time.Time
+	if wait > 0 {
+		timer := time.NewTimer(wait)
+		defer timer.Stop()
+		expired = timer.C
+	}
+	select {
+	case <-f.started:
+		return nil
+	case <-ctx.Done():
+		return ctx.Err()
+	case <-f.ctx.Done():
+		return caddyhttp.Error(http.StatusServiceUnavailable, frankenphp.ErrNotRunning)
+	case <-expired:
+		return caddyhttp.Error(http.StatusServiceUnavailable, frankenphp.ErrMaxWaitTimeExceeded)
+	}
+}
+
+func (f *FrankenPHPApp) reloadModule(name string) *FrankenPHPModule {
+	var target *FrankenPHPModule
+	for _, m := range f.modules {
+		if m.server == nil || m.reloadName != name {
+			continue
+		}
+		// A php_server may embed several handlers sharing one server.
+		if target != nil && target.server != m.server {
+			return nil
+		}
+		if target == nil {
+			target = m
+		}
+	}
+	return target
+}