feat: declared background workers + frankenphp_get_worker_handle() - #2543
Open
nicolas-grekas wants to merge 1 commit into
Open
feat: declared background workers + frankenphp_get_worker_handle()#2543nicolas-grekas wants to merge 1 commit into
nicolas-grekas wants to merge 1 commit into
Conversation
Background workers are long-lived non-HTTP PHP scripts declared via WithWorkerBackground() or the `background` flag in Caddyfile worker blocks. The script runs in a loop: it is re-run on cooperative exit (status 0) and restarted with a quadratic backoff on crash, failing hard on max_consecutive_failures during startup only. frankenphp_get_worker_handle() returns a stream over the read end of a per-thread stop pipe; draining the thread (shutdown, reboot, handler transition) closes the write end, so a script parked in stream_select wakes up and can exit gracefully. Background workers attach to a Server through the existing WithWorkerServerScope(); their name is mandatory (it is the script's identity, exposed as FRANKENPHP_WORKER) and lives in the global worker namespace, which the Caddy module already qualifies per server.
Contributor
Author
|
Should be considered after #2499 is merged of course! |
This was referenced Jul 20, 2026
AlliBalliBaba
pushed a commit
that referenced
this pull request
Aug 5, 2026
Follow-up to #2499, targeting `refactor/phpserver`. Four commits, each independently reviewable. Findings from a close read of the branch. Nothing here changes the direction of the refactor; the first two are the ones I would not want to ship. ### 1. The per-worker Mercure hub is dropped `assignMercureHub()` fills `workerConfig.options`, but the new `toWorkerOptions()` builds a fresh slice and never appends it, where `Start()` used to pass `w.options` directly. The field is now written and never read. `WithWorkerMercureHub()` appends `WithMercureHub()` to the worker request options, which is what lets a worker publish from its startup code, outside `frankenphp_handle_request()`. Request-scoped publishing kept working because the module carries `WithMercureHub()` in its own request options, which is why no test noticed. #2543 extends the same function and inherits the bug. ### 2. `FrankenPHPModule.server` can be nil in `ServeHTTP()` It is assigned by `FrankenPHPApp.Start()`, but caddy starts apps by ranging over a map, so the http app can begin serving first. The handler dereferenced it unconditionally, turning that window into a nil pointer panic where `main` returned a clean `ErrNotRunning`. Two neighbouring lifecycle issues are fixed in the same commit: modules left in `app.modules` by a config that failed to provision are registered again by the next reload (the app is a process singleton and `reset()` only runs at the end of `Start()`), and `match` in a global worker block was parsed then silently ignored, so it is now rejected at parse time as `docs/config.md` already documents. ### 3. A `Server` could not be registered twice `unregisterServers()` only flipped `isRegistered`, so the worker slices and maps kept the previous run's entries and a second `Init()` with the same `*Server` failed with `two workers in a server cannot have the same filename`. Caddy dodges this by building a fresh `NewServer` per `Start()`, but `docs/library.md` presents `NewServer` + `Init` as the library pattern with no hint the instance is single-use. Same commit: the `server_<idx>` default was written into `s.name` permanently, servers were marked registered about a hundred lines before `initWorkers()` and the thread setup (a request in that window found neither a worker nor a regular thread, so `activateServers()` is split out), and `WithWorkerMatcher()` without `WithWorkerServerScope()` was a silent no-op that still got path-matched via `globalWorkersByPath`, the opposite of what the matcher asked for. ### 4. Logger and prepared env Worker startup contexts hardcoded `globalLogger` even though the worker's server was at hand, so worker boot messages and worker stdout bypassed the per-`php_server` logger this branch introduces. The module used to append `WithRequestLogger()` to each worker's request options and that line is gone, so nothing else covered it. `go_register_server_variables()` merged the prepared env whenever the server had one, but `registerPreparedEnv()` only runs from `go_update_request_info()`, which returns early without a request. For a server-scoped extension worker handling a message the merge copied whatever the thread-local prepared env held from an earlier request. ### Tests Added: `toWorkerOptions()` keeps provisioned options, a `Server` re-registered after `Shutdown()` still serves its worker and keeps its name, and a request matcher without a server scope is rejected. Both modules pass `go vet` including test files. The runtime tests need CI: on my box (WSL2) per-thread engine bootstrap of an embed ZTS build is pathologically slow and linking needs dev libs I do not have, so every `Init()`-based test is unrunnable locally, including on unmodified base commits. ### Not included `worker.mercureHub` is assigned by `configureMercure()` and never read anywhere; removing the dead field touches the `nomercure` build-tag pair, so I left it alone. Say the word and I will fold it in.
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
1 participant
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.
Summary
Rewrite of #2398 on top of #2499, targeting the
refactor/phpserverbranch:the smallest useful slice of the background-worker work, now built on
Serverand the server names merged in #2529 instead of a parallelScopemechanism. A single commit on top of the base branch.
A worker declared with
WithWorkerBackground()/backgroundin theCaddyfile runs its script in a loop outside the HTTP request cycle. The
frankenphp_get_worker_handle()primitive hands the script a stream thatreaches EOF when FrankenPHP drains the worker, so it can park on
stream_select()and exit gracefully on shutdown, reboot or restart.What's in
frankenphp_get_worker_handle(): resource, closes when theworker is drained (the Go side closes the write end of a per-thread stop
pipe).
WithWorkerBackground(). Background workers attach to aServerthrough the existingWithWorkerServerScope();num >= 1isrequired (no lazy-start in this build) and the name is mandatory since it
is the script's identity (
$_SERVER['FRANKENPHP_WORKER']).backgroundWorkerThreadimplementsthreadHandlerandmirrors
workerThread's state machine: boot, re-run on cooperative exit(status 0, backoff reset), crash-restart with quadratic backoff, hard
failure on
max_consecutive_failuresduring startup only.backgroundflag inside worker blocks (php_server and global).nameis required,matchis rejected. Metrics/logs use the worker name,which feat: name Server instances and attribute workers to them聽#2529's qualification already keeps unique across
php_serverblocks, so two blocks can declare the same worker name.
What the rewrite changes vs #2398
Building on
Serverdeletes the parallel machinery the old PR carried andaddresses all review comments:
Scope,NextScope,SetScopeLabel, thescopeOwnersregistry andcaddy/scopelabel.goare gone;Serverand the feat: name Server instances and attribute workers to them聽#2529 name resolutionreplace them. This also removes the reported
scopeOwnersdata race andthe
m#<label>:m#<name>double-prefix in metric names. The feature diffis ~250 lines smaller than feat: declared background workers + frankenphp_get_worker_handle()聽#2398.
php_streamin a TLS slot: eachfrankenphp_get_worker_handle()call dups the read fd into a freshstream owned by its zval. No
GC_ADDREFwithout release, no danglingpointer across restarts or thread recycling, nothing to clean up in
frankenphp_update_local_thread_context()beyond the raw fds.-1return offrankenphp_set_background_worker_and_get_stop_fd_write()is handled:
Init()fails with a clear error during startup; paststartup the thread backs off and retries instead of letting the script
crash-loop on
frankenphp_get_worker_handle()exceptions.implementation.
drain()(thethreadHandlerseam that is declared but not called yeton this branch) is wired into
thread.shutdown()andrebootAllThreads(), so shutdowns and watch-triggered reboots wakeparked background workers instead of hitting the force-kill grace period.
frankenphp_handle_request()throws when called from a background worker (previously it would have hit
the
.(*workerThread)type assertion and crashed the process), andWithWorkerName()rejects requests targeting a background worker.ready once the script starts, and every stop path emits
StopWorker), sothe
readyWorkersgauge can't drift negative.Deferred (kept out for review surface)
frankenphp_ensure_background_worker()and lazy-start machinery.frankenphp_set_vars/frankenphp_get_vars).frankenphp_start_background_worker()runtime API(discussed in feat: declared background workers + frankenphp_get_worker_handle()聽#2398); the primitives here are compatible with it.
Test plan
TestBackgroundWorkerLifecycle: bg worker boots, touches sentinel,parks on the stop pipe,
Shutdown()returns within 10s.TestBackgroundWorkerCrashRestarts:exit(1)on first boot, therespawned run touches the "restarted" sentinel.
TestBackgroundWorkerOnServer: server-scoped worker inherits theserver env,
FRANKENPHP_WORKERcarries the name, HTTP requests on thesame server still serve.
TestBackgroundWorkerValidation: name required,num >= 1, globalname namespace, request matchers rejected.
TestWorkerBackgroundConfig/RequiresName/RejectsMatch(Caddyfile parsing, pass locally).
The runtime tests need CI to confirm: on my current box (WSL2), per-thread
engine bootstrap of any embed ZTS build is pathologically slow, so every
Init()-based test times out locally, including on unmodified base commits.