feat: Add configurable max_requests for PHP threads by nicolas-grekas · Pull Request #2292 · php/frankenphp

nicolas-grekas · 2026-03-21T15:17:22Z

PHP-FPM recycles worker processes after a configurable number of requests (pm.max_requests), preventing memory leaks from accumulating over time. FrankenPHP keeps PHP threads alive indefinitely, so any leak in PHP extensions (e.g. ZTS builds of profiling tools like Blackfire) or application code compounds over hours/days. In production behind reverse proxies like Cloudflare, this can lead to gradual resource exhaustion and eventually 502 errors with no visible warnings in logs.

This PR adds a max_requests option in the global frankenphp block that automatically restarts PHP threads after a given number of requests, fully cleaning up the thread's memory and state. It applies to both regular (module mode) and worker threads.

When a thread reaches the limit it exits the C thread loop, triggering a full cleanup including any memory leaked by extensions. A fresh thread is then booted transparently. Other threads continue serving requests during the restart.

This cannot be done from userland PHP: restarting a worker script from PHP only resets PHP-level state, not the underlying C thread-local storage where extension-level leaks accumulate. And in module mode (without workers), there is no userland loop to count requests at all.

Default is 0 (unlimited), preserving existing behavior.

Usage:

{
    frankenphp {
        max_requests 500
    }
}

Changes:

New max_requests Caddyfile directive in the global frankenphp block
New WithMaxRequests functional option
New Rebooting and RebootReady states in the thread state machine for restart coordination
Regular thread restart in threadregular.go
Worker thread restart in threadworker.go
Safe shutdown: shutdown() waits for in-flight reboots to complete before draining threads
Tests for both module and worker mode (sequential and concurrent), with debug log verification
Updated docs

henderkes · 2026-03-21T15:24:54Z

I would propose we either make this a global setting only or a php_server block setting only. I currently don't see why this should be a worker setting.

nicolas-grekas · 2026-03-21T16:38:28Z

See #280

As I wrote: this can be dealt with on the PHP side, but this is an ops concern, so it would make sense to have this in the Caddyfile, don't you think?

nicolas-grekas · 2026-03-21T17:06:06Z

Actually I had a closer look and realized we should do the same full reset on worker threads too - something userland PHP cannot do. Both worker and module mode now do full ZTS cleanup via ts_free_thread() when max_requests is reached.

That complements user-side refreshes that clean only application state.

nicolas-grekas · 2026-03-21T21:51:07Z

PR description updated, patch ready and green 🚀

henderkes · 2026-03-22T02:08:32Z

See #280

As I wrote: this can be dealt with on the PHP side, but this is an ops concern, so it would make sense to have this in the Caddyfile, don't you think?

I just meant that I don't think the setting should exist in more than one place in the Caddyfile config. I'm leaning towards it only being a php_server setting (which would affect all workers in that block too).

nicolas-grekas · 2026-03-22T09:19:55Z

What about having only frankenphp { max_requests } that applies to everything? That would work also for global workers.

henderkes · 2026-03-22T10:05:37Z

Would be fine, although I think that we generally want almost all options to be inside php_server blocks. I think workers should have never been in global scope either.

nicolas-grekas · 2026-03-22T10:19:17Z

BTW, what about defaulting to eg 100?

henderkes · 2026-03-22T10:30:05Z

I don't think we should. Default should stay 0 for performance and because most apps don't actually have memory issues in non-worker mode.

nicolas-grekas · 2026-03-22T15:57:10Z

Done, there's not a single max_requests under frankenphp, default to 0 = unlimited.

docs/config.md

maxrequests_regular_test.go

threadworker.go

henderkes · 2026-03-22T16:39:41Z

Can we get a quick vote on frankenphp wide vs per-php_server?

👍 for frankenphp, 👎 for php_server
@php/frankenphp-collaborators

nicolas-grekas · 2026-03-22T16:51:35Z

My reasoning for going with only frankenphp after your suggestion: the ini settings and list of enabled extensions is global. If there's a leak to accommodate for, it's thus global also.

nicolas-grekas · 2026-03-22T17:01:08Z

Thanks for the review @henderkes, should be all addressed now. (just waiting for the CI now)

henderkes · 2026-03-22T19:20:01Z

Thanks for the review @henderkes, should be all addressed now. (just waiting for the CI now)

Hmm, that's a good point.

henderkes

All good from my side now, but I'll look into properly testing for the restart log message tomorrow, if you don't get to it first.

Thanks for the addition and your work on Symfony!

threadworker.go

maxrequests_regular_test.go

maxrequests_test.go

nicolas-grekas · 2026-03-23T08:13:25Z

Thanks for the review, all addressed:

Added Rebooting state to internal/state/state.go
Removed needsRestart field, restartingThreads and shutdownInProgress atomics
waitForRequest/waitForWorkerRequest sets state.Rebooting before triggering restart
beforeScriptExecution handles Rebooting state to exit C loop
shutdown() waits for Rebooting threads to complete before proceeding
Restart goroutines check mainThread.state.Is(Ready) to abort during shutdown
Use testGet() instead of manual HTTP clients
Removed explicit timeouts, rely on test-level timeout
100% success required (not 95%)
Removed TestModuleMaxRequestsZeroIsUnlimited (didn't test much indeed)
Moved worker tests to worker_test.go
Deleted maxrequests_test.go

threadworker.go

threadregular.go

phpthread.go

AlliBalliBaba · 2026-03-24T19:28:45Z

frankenphp.go

+	regularThreadMu.Lock()
 	regularThreads = make([]*phpThread, 0, opt.numThreads-workerThreadCount)
+	regularThreadMu.Unlock()


lock here is not necessary

AlliBalliBaba · 2026-03-24T19:29:03Z

phpthread.go

+	if thread.state.Is(state.Rebooting) || thread.state.Is(state.RebootReady) {
+		thread.state.WaitFor(state.Ready, state.Inactive, state.Done)
+	}


check is unnecessary

AlliBalliBaba · 2026-03-24T19:30:40Z

threadregular.go

+		if !handler.state.CompareAndSwap(state.Ready, state.Rebooting) {
+			return ""
+		}
+		detachRegularThread(handler.thread)


detaching is unnecessary

AlliBalliBaba · 2026-03-24T19:30:58Z

threadworker.go


+		return handler.worker.fileName
+	case state.Rebooting:
+		handler.worker.detachThread(handler.thread)


detaching is unnecessary

AlliBalliBaba · 2026-03-24T19:31:54Z

threadworker.go

+// restartWorkerThread waits for the C thread to exit, then spawns a fresh one.
+func restartWorkerThread(thread *phpThread, worker *worker) {
+	thread.state.WaitFor(state.RebootReady)
+	thread.drainChan = make(chan struct{})


making a new drainchan is unnecessary

AlliBalliBaba · 2026-03-24T19:35:00Z

threadworker.go

+		handler.thread.updateContext(true)
+		handler.worker.attachThread(handler.thread)
+		if handler.worker.onThreadReady != nil {
+			handler.worker.onThreadReady(handler.thread.threadIndex)
+		}
+		setupWorkerScript(handler, handler.worker)
 		return handler.worker.fileName


Suggested change

handler.thread.updateContext(true)

handler.worker.attachThread(handler.thread)

if handler.worker.onThreadReady != nil {

handler.worker.onThreadReady(handler.thread.threadIndex)

}

setupWorkerScript(handler, handler.worker)

return handler.worker.fileName

return handler.beforeScriptExecution()

AlliBalliBaba · 2026-03-24T19:36:14Z

threadregular.go

+	case state.RebootReady:
+		handler.requestCount = 0
+		handler.state.Set(state.Ready)
+		attachRegularThread(handler.thread)


attaching is unnecessary

AlliBalliBaba · 2026-03-24T19:36:39Z

threadregular.go

+// restartRegularThread waits for the C thread to exit, then spawns a fresh one.
+func restartRegularThread(thread *phpThread) {
+	thread.state.WaitFor(state.RebootReady)
+	thread.drainChan = make(chan struct{})


making new drain chan is unnecessary

AlliBalliBaba · 2026-03-24T19:38:03Z

threadworker.go

+		handler.state.CompareAndSwap(state.Ready, state.Rebooting)
+		return false, nil


Suggested change

handler.state.CompareAndSwap(state.Ready, state.Rebooting)

return false, nil

if (handler.state.CompareAndSwap(state.Ready, state.Rebooting){

return false, nil

}

nicolas-grekas force-pushed the max_requests branch from 45cffba to bebcd7a Compare March 21, 2026 15:18

nicolas-grekas force-pushed the max_requests branch 3 times, most recently from b30002c to 9f13975 Compare March 21, 2026 17:03

nicolas-grekas force-pushed the max_requests branch 2 times, most recently from 13baffd to ae8d6ed Compare March 21, 2026 21:28

nicolas-grekas force-pushed the max_requests branch from ae8d6ed to c635f17 Compare March 22, 2026 15:55

henderkes reviewed Mar 22, 2026

View reviewed changes

docs/config.md Outdated Show resolved Hide resolved

maxrequests_regular_test.go Outdated Show resolved Hide resolved

maxrequests_regular_test.go Show resolved Hide resolved

threadworker.go Outdated Show resolved Hide resolved

nicolas-grekas force-pushed the max_requests branch 2 times, most recently from 2b5961b to f2a3936 Compare March 22, 2026 16:59

nicolas-grekas force-pushed the max_requests branch from f2a3936 to e29b6a9 Compare March 22, 2026 17:37

henderkes approved these changes Mar 22, 2026

View reviewed changes

nicolas-grekas force-pushed the max_requests branch from e29b6a9 to 5c53ffb Compare March 22, 2026 20:43