feat: declared background workers + frankenphp_get_worker_handle() - #2617
feat: declared background workers + frankenphp_get_worker_handle()#2617nicolas-grekas wants to merge 6 commits into
Conversation
|
Please rewrite the PR description to not be LLM slop reasoning with itself about what it did and why. I've tried reading this three times and I just can't. |
|
Sure, I'll let you know when I'm done, for now I just let it do the rebase 😅 |
henderkes
left a comment
There was a problem hiding this comment.
What happens here when a global background worker and a php_server scoped background worker share the same name and are both eligible for the same source file?
| } | ||
|
|
||
| /* Dup so the returned stream owns its fd: closing the stream (or request | ||
| * shutdown destroying it) never touches worker_stop_fds[0], which stays |
There was a problem hiding this comment.
aren't we leaking fd's on every restart then?
henderkes
left a comment
There was a problem hiding this comment.
found another one, anyway, have you tested this on windows?
There was a problem hiding this comment.
Pull request overview
Adds declared background PHP workers with graceful stop-stream handling and Caddy configuration support.
Changes:
- Adds background-worker lifecycle, validation, and thread allocation.
- Exposes
frankenphp_get_worker_handle(). - Adds Caddy integration, documentation, fixtures, and tests.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
worker.go |
Registers and validates background workers. |
threadbackgroundworker.go |
Implements background-worker lifecycle. |
requestoptions.go |
Rejects background workers for HTTP requests. |
phpthread.go |
Drains handlers during shutdown and transitions. |
phpmainthread.go |
Drains handlers during reboot. |
options.go |
Adds WithWorkerBackground(). |
frankenphp.go |
Reserves background-worker threads. |
frankenphp.c |
Implements stop pipes and PHP API. |
frankenphp.h |
Declares C primitives. |
frankenphp.stub.php |
Declares the PHP function. |
frankenphp_arginfo.h |
Registers generated arginfo. |
docs/config.md |
Documents background configuration. |
caddy/workerconfig.go |
Parses background worker blocks. |
caddy/config_test.go |
Tests Caddy parsing and validation. |
bgworker_test.go |
Tests lifecycle, restart, scope, and validation. |
testdata/bgworker/basic.php |
Provides lifecycle fixture. |
testdata/bgworker/crash.php |
Provides restart fixture. |
testdata/bgworker/early-return.php |
Provides startup-failure fixture. |
testdata/bgworker/named.php |
Provides named-worker fixture. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (is_background_worker) { | ||
| is_background_worker = false; | ||
| frankenphp_worker_close_stop_fds(); | ||
| } |
| if handler, ok := phpThreads[threadIndex].handler.(*backgroundWorkerThread); ok && !handler.reachedHandle { | ||
| handler.reachedHandle = true | ||
| metrics.ReadyWorker(handler.worker.name) |
| if handler.state.Is(state.TransitionComplete) { | ||
| handler.state.Set(state.Ready) | ||
| } |
| // background workers reserve their thread budget separately so they | ||
| // don't count against the HTTP-oriented admission checks below; the | ||
| // bump is applied on top of the calculated totals at the end | ||
| reservedThreads := 0 | ||
| defer func() { | ||
| if err != nil { | ||
| return | ||
| } | ||
| opt.numThreads += reservedThreads | ||
| if opt.maxThreads > 0 { | ||
| // in auto mode (maxThreads < 0), the resolved value is floored | ||
| // to numThreads later, which already includes the reservation | ||
| opt.maxThreads += reservedThreads | ||
| } | ||
| numWorkers += reservedThreads | ||
| }() | ||
|
|
||
| for i, w := range opt.workers { | ||
| if w.isBackgroundWorker { | ||
| if w.num < 1 { | ||
| return 0, fmt.Errorf("background worker %q must declare num >= 1", w.name) | ||
| } | ||
| metrics.TotalWorkers(w.name, w.num) | ||
| reservedThreads += w.num | ||
|
|
||
| continue | ||
| } | ||
|
|
||
| if w.num <= 0 { | ||
| // https://github.com/php/frankenphp/issues/126 | ||
| opt.workers[i].num = maxProcs | ||
| } |
There was a problem hiding this comment.
Hmm I don't remember the specific reason why background workers are treated separately here. Wouldn't it make sense to just do this and count it into the general pool, like other workers:
if w.num <= 0 {
if w.isBackgroundWorker {
opt.workers[i].num = 1
} else {
opt.workers[i].num = maxProcs
}
}Worker thread count is already added on top of the general thread count. It would only overflow in case someone sets a general cap on global threads, in which case it should probably still honor that cap.
| $stream = frankenphp_get_worker_handle(); | ||
| $read = [$stream]; | ||
| $write = null; | ||
| $except = null; | ||
| stream_select($read, $write, $except, null); |
There was a problem hiding this comment.
It looks like currently the handle is only used for shutdown. IIRC in the future you'd also want to use the handle to send messages or even requests.
Would it maybe be cleaner to have a separate handle for each? Makes the api look more like we're selecting over different channels, in other words:
frankenphp_get_shutdown_handle(); # instead of frankenphp_get_worker_handle
frankenphp_get_message_handle(); # future scope: can return a dedicated message
frankenphp_get_request_handle(); # future scope: can return a dedicated request objectBackground 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.
…guard - setHandler() closed drainChan without calling the old handler's drain(), so a background script parked in stream_select slept through handler transitions (autoscaling, thread recycling) until the force-kill grace period. Drain first, guarding against the nil handler of a fresh thread. - Wrap the two zend_unset_timeout() calls in #ifdef ZEND_MAX_EXECUTION_TIMERS, matching every other timer call site; on builds without max-execution-timers (macOS) the setitimer path must stay untouched. - A background script that exited 0 without ever fetching its handle was treated as a cooperative exit and respawned immediately: an early return spun in a tight loop. Fetching the handle via frankenphp_get_worker_handle() is now the "reached steady state" marker, the background analog of HTTP workers reaching frankenphp_handle_request(): exits without it count as boot failures (backoff, max_consecutive_failures fails Init during startup). ReadyWorker moves from script start to handle fetch, so the ready gauge only counts scripts that actually reached their park point and stays balanced with StopReasonBootFailure. - Spell out the fd lifecycle at the dup site: dup'ed fds die with their streams at request shutdown, the read end is closed by the next setup or thread recycle, the write end by the Go side on every exit path.
Worker names were kept in one process-wide map, a leftover from before Server existed: path matching already moved to Server.workersByPath plus globalWorkersByPath, names had not. The Caddy module papered over it by renaming colliding workers to "<server>:<name>" or "<name>_N" at startup, which made $_SERVER['FRANKENPHP_WORKER'] of a background worker depend on what the other blocks declare. Names now follow paths: Server.workersByName and globalWorkersByName, unique per scope, so two php_server blocks may each declare "queue". The script sees the declared name and WithWorkerName() resolves within the request's server before falling back to the global workers. Metrics and logs still need a process-wide identity: worker.qualifiedName is "<server name>:<name>" for scoped workers and the bare name otherwise. Several blocks can resolve to the same server name (same host, same listen address), so registerServers() suffixes duplicates. The registration checks move next to the maps they guard (addGlobalWorker and Server.addWorker), TotalWorkers is reported from initWorkers() where the name is resolved, and createUniqueWorkerName() is deleted with its tests. The Caddyfile syntax is unchanged.
464a881 to
c3f8946
Compare
- Background threads were marked Ready in setupScript(), before the script ran. initWorkers() waits for that state and then clears startupFailChan, so a script failing before fetching its handle never failed Init(): the early-return test got no error, and the next Init() saw ErrAlreadyStarted. Mirror HTTP workers, which stay in TransitionComplete until frankenphp_handle_request(): Ready is now published from go_frankenphp_background_worker_ready(), and the setup retry loop treats TransitionComplete as still booting. - The stop pipe's read end is plain thread-local state that only frankenphp_update_local_thread_context() released, on recycle. On shutdown, reboot or an unhealthy exit php_thread() left its loop without it, leaking one fd per background thread per exit. Close it in the thread teardown as well. - The ready_workers help text and the StopReasonBootFailure comment still named frankenphp_handle_request() as the only ready point.
Fetching the handle was the ready point, but nothing forces a script to fetch it after bootstrapping. A script that fetches first and then crashes while booting was counted as a crash rather than a boot failure, so Init() succeeded and the worker quietly restart-looped, and the ready_workers gauge counted scripts that had not finished booting. Waiting on the handle is the actual steady state and comes after the bootstrap by construction, the way frankenphp_handle_request() does for HTTP workers. The handle now carries its own stream ops, copied from the plain-file ops at MINIT, whose select cast and read report the worker ready once per run through the existing go_frankenphp_background_worker_ready(). The Go side mirrors workerThread with an isBootingScript flag: an exit before the first wait is a boot failure, whether or not the handle was fetched. fetch-no-wait.php covers that case.
9d138a7 to
051b675
Compare
The handle was the read end of a pipe. On Windows, PHP's stream_select() is its own php_select(): sockets pass straight through to Winsock, but before 8.5 any other handle is reported as always ready, so a script parking on the pipe spun. 8.5 added a PeekNamedPipe path, which is why CI passed there, but older versions have to work too. The stop channel is now a socket pair on every platform (AF_UNIX, or PHP's loopback AF_INET emulation on Windows, which only accepts that family), so stream_select() takes the version-independent socket path everywhere and scripts stay identical. Both ends are marked non-inheritable: a child spawned by the script holding the Go side's end would keep the script's end from ever reaching EOF. Handles no longer own a duplicated descriptor. php_sockop_close() on Windows calls shutdown(SHUT_RD) before closing, which acts on the underlying socket and would reach every other handle, so the handle ops now close with close_handle=0: each call still returns a fresh stream, the socket belongs to the thread and is closed at the next run setup or on thread exit. The stream's read timeout is set to infinite so that a blocking read parks without default_socket_timeout wake-ups. The Go side holds the socket as an int64, since a Windows SOCKET is pointer-sized.
051b675 to
59bf8fd
Compare
A worker marked
backgroundruns its script in a loop outside the HTTP request cycle.frankenphp_get_worker_handle()hands the script a stream that reaches EOF when FrankenPHP drains the worker, so it can park onstream_select()or a blocking read and exit gracefully on shutdown, reboot or restart.PHP API
frankenphp_get_worker_handle(): resource, a stream that reaches EOF when the worker is drained: the Go side closes its end of a per-thread socket pair. A socket pair rather than a pipe so thatstream_select()takes the plain socket path on Windows too, where PHP'sphp_select()only waits properly on sockets before 8.5. Throws when called outside a background worker. Each call returns a fresh stream over the same socket; streams don't own it, so closing one never affects another, and the read timeout is infinite so a blocking read parks as well asstream_select()does.Go API
WithWorkerBackground(). Background workers attach to aServerthrough the existingWithWorkerServerScope().num >= 1is required (no lazy-start in this build) and the name is mandatory, since it is the script's identity and is exposed as$_SERVER['FRANKENPHP_WORKER'].Lifecycle
backgroundWorkerThreadimplementsthreadHandlerand mirrorsworkerThread's state machine: boot, re-run on cooperative exit (status 0, backoff reset), crash-restart with quadratic backoff, hard failure onmax_consecutive_failuresduring startup only.A worker counts as ready once it first waits on its handle (
stream_select()or a blocking read), the background analog offrankenphp_handle_request():Init()waits for that point,ready_workerscounts from it, and an exit before it is a boot failure whether or not the handle was fetched.drain()is wired intothread.shutdown()andrebootAllThreads(), so shutdowns and watch-triggered reboots wake parked background workers instead of waiting out the force-kill grace period.Caddy
backgroundflag inside worker blocks, in bothphp_serverand global ones.nameis required,matchis rejected. Metrics and logs use the worker name, which server-qualified naming already keeps unique acrossphp_serverblocks, so two blocks may declare the same worker name.Worker names
Names are scoped the way paths already are: unique within a
php_server(or among global workers), so two blocks may each declare a worker namedqueue. A background script sees the declared name in$_SERVER['FRANKENPHP_WORKER'], andWithWorkerName()resolves within the request's server before the global workers. Metrics and logs need a process-wide identity, so a server-scoped worker is reported there as<server name>:<name>; server names get a numeric suffix when two blocks resolve to the same one. The collision-driven renaming in the Caddy module is gone.Deferred
frankenphp_ensure_background_worker()and lazy-start machineryfrankenphp_set_vars/frankenphp_get_vars)frankenphp_start_background_worker()runtime API; the primitives here are compatible with itTests
TestBackgroundWorkerLifecycle: boots, touches its sentinel, parks on the stop pipe,Shutdown()returns within 10sTestBackgroundWorkerCrashRestarts:exit(1)on first boot, the respawned run touches the "restarted" sentinelTestBackgroundWorkerOnServer: inherits the server env,FRANKENPHP_WORKERcarries the declared name while a global worker reuses it, HTTP requests on the same server still serveTestBackgroundWorkerValidation: name required,num >= 1, names unique within a server, request matchers rejected, an early return or a fetch without a wait fails startupTestWorkerBackgroundConfig/RequiresName/RejectsMatch: Caddyfile parsingSupersedes #2543 and #2398.