fix: serve requests reaching the previous configuration during reloads

caddy/admin_test.go

@@ -540,6 +540,89 @@ func TestRejectedReloadWithAModuleWorkerKeepsThePreviousSiteServing(t *testing.T
 	require.Equal(t, servedBefore+1, countedRequests(t, workerURL))
 }
 
+// requests still routed by the previous configuration must reach the new workers once booted
+func TestReloadKeepsServingWhileWorkersBoot(t *testing.T) {
+	config := `
+	{
+		skip_install_trust
+		admin localhost:2999
+		http_port ` + testPort + `
+	}
+
+	localhost:` + testPort + ` {
+		php_server {
+			root ../testdata
+			worker {
+				file worker-slow-boot.php
+				num 1
+			}
+		}
+	}
+	`
+
+	tester := caddytest.NewTester(t)
+	initServer(t, tester, config, "caddyfile")
+
+	workerURL := "http://localhost:" + testPort + "/worker-slow-boot.php"
+	client := &http.Client{Timeout: 3 * time.Second}
+	done := make(chan struct{})
+	var (
+		wg       sync.WaitGroup
+		mu       sync.Mutex
+		statuses = map[int]int{}
+	)
+	stop := sync.OnceFunc(func() {
+		close(done)
+		wg.Wait()
+	})
+	t.Cleanup(stop)
+
+	// one request per tick: a request stuck on a draining worker must not stop the probing
+	wg.Go(func() {
+		ticker := time.NewTicker(5 * time.Millisecond)
+		defer ticker.Stop()
+
+		for {
+			select {
+			case <-done:
+				return
+			case <-ticker.C:
+			}
+
+			wg.Go(func() {
+				resp, err := client.Get(workerURL)
+				if err != nil {
+					return
+				}
+				_, _ = io.Copy(io.Discard, resp.Body)
+				_ = resp.Body.Close()
+
+				mu.Lock()
+				statuses[resp.StatusCode]++
+				mu.Unlock()
+			})
+		}
+	})
+
+	for range 3 {
+		r, err := http.NewRequest("POST", "http://localhost:2999/load", strings.NewReader(config))
+		require.NoError(t, err)
+		r.Header.Set("Content-Type", "text/caddyfile")
+		// reload even though the configuration did not change
+		r.Header.Set("Cache-Control", "must-revalidate")
+		resp, err := http.DefaultClient.Do(r)
+		require.NoError(t, err)
+		_, _ = io.Copy(io.Discard, resp.Body)
+		require.NoError(t, resp.Body.Close())
+		require.Equal(t, http.StatusOK, resp.StatusCode)
+	}
+
+	stop()
+
+	require.NotZero(t, statuses[http.StatusOK])
+	assert.Equal(t, map[int]int{http.StatusOK: statuses[http.StatusOK]}, statuses)
+}
+
 // the number of requests testdata/worker-with-counter.php has served
 func countedRequests(t *testing.T, workerURL string) int {
 	t.Helper()

caddy/app.go

@@ -23,6 +23,9 @@ import (
 var (
 	options   []frankenphp.Option
 	optionsMU sync.RWMutex
+
+	// the app whose servers are registered, or about to be
+	activeApp atomic.Pointer[FrankenPHPApp]
 )
 
 // EXPERIMENTAL: RegisterWorkers provides a way for extensions to register frankenphp.Workers
@@ -180,10 +183,15 @@ func (f *FrankenPHPApp) Start() error {
 		return err
 	}
 
+	// requests still reaching the previous configuration are forwarded to this one
+	activeApp.Store(f)
+
 	// if FrankenPHP is currently running, shut it down first
 	// this will happen in admin API reloads and caddy tests
 	frankenphp.Shutdown()
 	if err := frankenphp.Init(f.opts...); err != nil {
+		activeApp.CompareAndSwap(f, nil)
+
 		return err
 	}
 
@@ -193,6 +201,8 @@ func (f *FrankenPHPApp) Start() error {
 }
 
 func (f *FrankenPHPApp) Stop() error {
+	activeApp.CompareAndSwap(f, nil)
+
 	if f.logger.Enabled(f.ctx, slog.LevelInfo) {
 		f.logger.LogAttrs(f.ctx, slog.LevelInfo, "FrankenPHP stopped 🐘")
 	}

caddy/module.go

@@ -185,14 +185,9 @@ 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)
-		}
+func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error {
+	if !waitForStart(f.app) {
+		return caddyhttp.Error(http.StatusServiceUnavailable, frankenphp.ErrNotRunning)
 	}
 
 	ctx := r.Context()
@@ -226,6 +221,9 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 	}
 
 	err := f.server.ServeHTTP(w, r, opts...)
+	if errors.Is(err, frankenphp.ErrNotRunning) {
+		return f.serveWithReplacement(w, r, next)
+	}
 
 	if _, rejected := errors.AsType[frankenphp.ErrRejected](err); err != nil && !rejected {
 		return caddyhttp.Error(http.StatusInternalServerError, err)
@@ -234,6 +232,49 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
 	return nil
 }
 
+// waitForStart stalls the request for up to 10 seconds if FrankenPHP has not started yet
+func waitForStart(app *FrankenPHPApp) bool {
+	if app.hasStarted.Load() {
+		return true
+	}
+
+	select {
+	case <-app.started:
+		return true
+	case <-time.After(10 * time.Second):
+		return false
+	}
+}
+
+// serveWithReplacement hands the request to the configuration Caddy started before this one stopped routing
+func (f *FrankenPHPModule) serveWithReplacement(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error {
+	notRunning := caddyhttp.Error(http.StatusServiceUnavailable, frankenphp.ErrNotRunning)
+
+	app := activeApp.Load()
+	if app == nil || app == f.app || !waitForStart(app) {
+		return notRunning
+	}
+
+	var target *FrankenPHPModule
+	for _, m := range app.modules {
+		if m.server == nil || m.server.Name() != f.server.Name() {
+			continue
+		}
+
+		// modules sharing a server are interchangeable, distinct servers with the same name are not
+		if target != nil && target.server != m.server {
+			return notRunning
+		}
+		target = m
+	}
+
+	if target == nil {
+		return notRunning
+	}
+
+	return target.ServeHTTP(w, r, next)
+}
+
 // UnmarshalCaddyfile implements caddyfile.Unmarshaler.
 func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	for d.Next() {

testdata/worker-slow-boot.php

@@ -0,0 +1,9 @@
+<?php
+
+// simulates a framework bootstrap, so that reloads take a while to boot workers
+usleep(300000);
+
+while (frankenphp_handle_request(static function (): void {
+    echo 'ok';
+})) {
+}