Skip to content

fix: serve requests reaching the previous configuration during reloads - #2705

Closed
dunglas wants to merge 1 commit into
mainfrom
fix/reload-forward-to-new-server
Closed

dunglas wants to merge 1 commit into
mainfrom
fix/reload-forward-to-new-server

Conversation

@dunglas

@dunglas dunglas commented Oct 10, 2026

Copy link
Copy Markdown
Member

Fixes #2698, a regression in 1.13.0.

Caddy starts the new configuration before it stops the previous one. Since #2499, each php_server has its own frankenphp.Server. The new app's Start() calls frankenphp.Shutdown(), which unregisters the servers the previous HTTP server still routes to. Until Caddy stops that HTTP server, every request it handles gets ErrNotRunning, answered with a 500. The window covers the whole worker boot (Laravel Octane: about 20 failed requests per reload).

How this fixes it:

  • The app being started is stored in activeApp before Shutdown().
  • When a module gets ErrNotRunning, it waits for that app to start, using the existing 10s stall. It then hands the request to the new module serving the same server name, as 1.12.7 effectively did by routing to the current workers.
  • Requests that can't be forwarded get a 503 instead of a 500: no matching server, the name is ambiguous, or the start timed out.

This is a smaller alternative to #2648. It doesn't replay the request through the Caddy handler chain and doesn't need to capture the body.

Not covered: shutdown() drains PHP threads before it unregisters the servers. A request queued on a draining worker during that window can hang, as on main. This is the side note in the issue, and a follow-up should reject those requests with ErrNotRunning so they get forwarded as well.

The new test sends requests in a loop during three reloads. It fails on main (e.g. 186 脳 500, 5 脳 200) and passes with the fix.

Caddy starts the new configuration before it stops the previous one. Since
each php_server got its own frankenphp.Server, the new app's Start()
unregisters the servers the previous HTTP server still routes to, so every
request it handles until it stops fails with ErrNotRunning, answered with a
500.

Such requests now wait for the new configuration to start and are handed to
its module serving the same server name. Requests that cannot be forwarded get
a 503 instead of a 500.

Fixes #2698
@dunglas

dunglas commented Oct 10, 2026

Copy link
Copy Markdown
Member Author

Closing in favor of #2700, which fixes #2698 in the core library and also covers requests queued on draining workers. The reload test from this PR passes on 2700's branch; it would be worth adding there.

@dunglas dunglas closed this Oct 10, 2026
@dunglas dunglas mentioned this pull request Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant