fix: keep PHP running until Caddy finishes graceful shutdown - #2645
Draft
nesl247 wants to merge 1 commit into
Draft
fix: keep PHP running until Caddy finishes graceful shutdown#2645nesl247 wants to merge 1 commit into
nesl247 wants to merge 1 commit into
Conversation
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.
Contributor
|
Makes sense 👍 . Could you maybe also test that a worker script is still able to do graceful shutdown, eg: # worker.php
while(frankenphp_handle_request($handler)){
}
# Make sure this code here still runs in a test, for example by creating a temp file
shutdownlogic(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2644.
Caddy stops its apps without dependency ordering. Calling
frankenphp.Shutdown()fromFrankenPHPApp.Stop()can therefore stop PHP while the HTTP app is still accepting requests duringshutdown_delay, producing HTTP 500 responses.Register a single process-global
caddy.OnExitcallback so PHP stays available until all Caddy apps have stopped, including the HTTP shutdown delay and in-flight request draining. Keep the existing reload shutdown path unchanged.Add a process-level regression to the existing Linux test workflow, using the CLI it already builds. Five fresh processes exercise the unordered stop path. The test verifies PHP responses during the shutdown delay, an in-flight response spanning listener closure, actual PHP teardown, and clean process exit. A bounded release handshake avoids waiting for a fixed long-running request.
Validated against current
main(daead870980d2aba0c2df76cbea880b5a608ff0b) on Linux ARM64 with PHP 8.5.9 and Go 1.27.1:mainafter SIGTERM.reload_test.shintegration check passes.The full PHP-version matrix and worker-mode shutdown were not run locally. The regression is wired into the existing PHP 8.2–8.5 Linux jobs. Feedback on the process-exit lifecycle hook is welcome.