From 93ad9b9b941caa81e974914000022f8ab37f27ff Mon Sep 17 00:00:00 2001 From: Alexander Kim Date: Fri, 11 Sep 2026 15:39:58 -0400 Subject: [PATCH] fix: keep PHP running until Caddy finishes graceful shutdown Caddy stops apps without dependency ordering. Shutting down PHP from the FrankenPHP app Stop method can break requests while HTTP remains open. Register one global OnExit callback after all apps have drained instead, while preserving the existing reload shutdown path. Add a process-level regression for service during shutdown_delay, in-flight request draining, PHP teardown, and successful process exit. Run it in the existing Linux PHP matrix using the already-built CLI. Validation on current main with Linux ARM64, PHP 8.5.9 and Go 1.27.1: patched regression and existing reload integration pass; unpatched main fails the regression. Go formatting, Bash syntax, ShellCheck and actionlint pass. Full PHP-version matrix and worker-mode shutdown not run locally. --- .github/workflows/tests.yaml | 2 + caddy/app.go | 7 -- caddy/caddy.go | 8 +++ shutdown_test.sh | 134 +++++++++++++++++++++++++++++++++++ 4 files changed, 144 insertions(+), 7 deletions(-) create mode 100755 shutdown_test.sh diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 8b0759f7a6..10e98474a2 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -75,6 +75,8 @@ jobs: run: sudo ../caddy/frankenphp/frankenphp start - name: Run integrations tests run: ./reload_test.sh + - name: Test graceful shutdown + run: ./shutdown_test.sh - name: Lint Go code uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 if: matrix.php-versions == '8.5' diff --git a/caddy/app.go b/caddy/app.go index fcee129180..469b8194ef 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -162,13 +162,6 @@ func (f *FrankenPHPApp) Stop() error { f.logger.LogAttrs(f.ctx, slog.LevelInfo, "FrankenPHP stopped 🐘") } - // attempt a graceful shutdown if caddy is exiting - // note: Exiting() is currently marked as 'experimental' - // https://github.com/caddyserver/caddy/blob/e76405d55058b0a3e5ba222b44b5ef00516116aa/caddy.go#L810 - if caddy.Exiting() { - frankenphp.Shutdown() - } - // reset global options optionsMU.Lock() options = nil diff --git a/caddy/caddy.go b/caddy/caddy.go index 24c5011900..f697cb6a99 100644 --- a/caddy/caddy.go +++ b/caddy/caddy.go @@ -4,11 +4,13 @@ package caddy import ( + "context" "fmt" "time" "github.com/caddyserver/caddy/v2" "github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile" + "github.com/dunglas/frankenphp" ) const ( @@ -22,6 +24,12 @@ const ( ) func init() { + // Keep PHP available until every Caddy app has finished stopping, including + // the HTTP server's shutdown delay and in-flight request draining. + caddy.OnExit(func(context.Context) { + frankenphp.Shutdown() + }) + caddy.RegisterModule(&FrankenPHPApp{}) caddy.RegisterModule(&FrankenPHPModule{}) caddy.RegisterModule(&FrankenPHPAdmin{}) diff --git a/shutdown_test.sh b/shutdown_test.sh new file mode 100755 index 0000000000..707e9c685c --- /dev/null +++ b/shutdown_test.sh @@ -0,0 +1,134 @@ +#!/usr/bin/env bash +# Exercise process shutdown against the locally built FrankenPHP CLI. +set -euo pipefail + +frankenphp_binary=${FRANKENPHP_BINARY:-./caddy/frankenphp/frankenphp} + +server_pid= +slow_pid= +directory= + +cleanup() { + for pid in "$slow_pid" "$server_pid"; do + if [[ -n "$pid" ]] && kill -0 "$pid" 2>/dev/null; then + kill -KILL "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + fi + done +} + +on_exit() { + local status=$? + if (( status != 0 )) && [[ -n "$directory" && -f "$directory/server.log" ]]; then + cat "$directory/server.log" >&2 + fi + cleanup + exit "$status" +} +trap on_exit EXIT + +fail() { echo "FAIL: $*" >&2; exit 1; } +request() { + local status error=0 + status=$(curl --silent --max-time 1 --output "$2" --write-out '%{http_code}' \ + "http://127.0.0.1:18080$1") || error=$? + case "$error" in + 0) printf '%s' "$status" ;; + 7) printf 'closed' ;; + # An accepted connection can close while HTTP shutdown begins. + 52|56) printf 'closing' ;; + *) printf 'error-%s' "$error" ;; + esac +} + +# Fresh processes exercise Caddy's non-deterministic app shutdown order. +for attempt in {1..5}; do + directory=$(mktemp -d) + cat > "$directory/Caddyfile" < "$directory/index.php" <<'PHP' += $deadline) { + http_response_code(504); + exit("Request was not released after HTTP draining\n"); + } + usleep(10_000); + clearstatcache(); + } +} +echo "ok\n"; +PHP + + "$frankenphp_binary" run --config "$directory/Caddyfile" > "$directory/server.log" 2>&1 & + server_pid=$! + deadline=$((SECONDS + 20)) + until [[ "$(request / "$directory/response")" == 200 ]] && [[ "$(cat "$directory/response")" == ok ]]; do + kill -0 "$server_pid" 2>/dev/null || fail 'Server exited before becoming ready' + (( SECONDS < deadline )) || fail 'PHP did not become ready' + sleep 0.05 + done + + curl --silent --show-error --fail --max-time 15 \ + 'http://127.0.0.1:18080/?slow=1' > "$directory/slow-response" & + slow_pid=$! + deadline=$((SECONDS + 10)) + until [[ -f "$directory/started" ]]; do + (( SECONDS < deadline )) || fail 'Slow PHP request did not start' + sleep 0.02 + done + kill -0 "$slow_pid" 2>/dev/null || fail 'Slow request finished before SIGTERM' + kill -TERM "$server_pid" + + served_during_delay=false + drained_inflight=false + deadline=$((SECONDS + 20)) + while kill -0 "$server_pid" 2>/dev/null; do + (( SECONDS < deadline )) || fail 'Server did not exit within the shutdown budget' + health=$(request /health /dev/null) + status=$(request / "$directory/response") + [[ "$health" != error-* ]] || fail "Health request failed with curl exit code ${health#error-}" + [[ "$status" != error-* ]] || fail "PHP request failed with curl exit code ${status#error-}" + if [[ "$status" != closed && "$status" != closing ]]; then + [[ "$status" == 200 ]] || fail "PHP returned HTTP $status during shutdown" + [[ "$(cat "$directory/response")" == ok ]] || fail 'PHP returned an incomplete response' + if [[ "$health" == 503 ]]; then served_during_delay=true; fi + elif [[ "$status" == closed && "$served_during_delay" == true ]] && kill -0 "$slow_pid" 2>/dev/null; then + drained_inflight=true + touch "$directory/release" + fi + sleep 0.02 + done + wait "$server_pid" || fail 'Server did not exit successfully' + server_pid= + grep -Fq '"msg":"FrankenPHP shut down"' "$directory/server.log" || fail 'PHP shutdown did not complete' + wait "$slow_pid" || fail 'In-flight PHP request failed' + slow_pid= + [[ "$(cat "$directory/slow-response")" == ok ]] || fail 'In-flight response was incomplete' + [[ "$served_during_delay" == true ]] || fail 'No successful PHP request during shutdown_delay' + [[ "$drained_inflight" == true ]] || fail 'In-flight request did not span HTTP draining' + echo "FrankenPHP graceful shutdown: $attempt/5 passed" +done