fix: throttle opcache_reset()-triggered thread pool reboots

frankenphp.go

@@ -784,9 +784,81 @@ func go_is_context_done(threadIndex C.uintptr_t) C.bool {
 
 //export go_schedule_opcache_reset
 func go_schedule_opcache_reset(threadIndex C.uintptr_t) {
-	if mainThread != nil {
-		go mainThread.rebootAllThreads()
+	// Capture the current *phpMainThread once: this goroutine can outlive
+	// this specific instance (e.g. Shutdown() runs while it's still holding
+	// the trailing cooldown below). mainThread is reassigned to a brand new
+	// instance by the next Init(), and phpThreads is reset with it; a stale
+	// goroutine that read the package-level mainThread var again later would
+	// operate on a different, unrelated, possibly concurrently-running
+	// instance instead of quietly becoming a no-op.
+	mt := mainThread
+	if mt == nil {
+		return
 	}
+
+	// Application code (e.g. some WordPress plugins) can call opcache_reset()
+	// on every request, and the same hook fires on an internal opcache
+	// restart (OOM, etc.). Since a reboot pauses request handling for every
+	// thread, coalesce calls that arrive within the cooldown into a single
+	// reboot instead of pausing the whole pool on every call.
+	//
+	// A call inside the cooldown must still result in a reboot once the
+	// cooldown ends, not be dropped: frankenphp_opcache_reset() always
+	// reports success to PHP regardless of what happens here, and a dropped
+	// OOM-triggered restart would leave the accelerator disabled until
+	// threads happen to reboot for an unrelated reason. opcacheResetScheduled
+	// coalesces any calls arriving while one is already pending, in flight,
+	// or in its post-reboot cooldown hold into that single reboot, rather
+	// than silently discarding them.
+	mt.opcacheResetMu.Lock()
+	if mt.opcacheResetScheduled {
+		mt.opcacheResetMu.Unlock()
+		return
+	}
+	mt.opcacheResetScheduled = true
+	wait := opcacheResetCooldown - time.Since(mt.lastOpcacheResetAt)
+	mt.opcacheResetMu.Unlock()
+
+	go func() {
+		defer func() {
+			mt.opcacheResetMu.Lock()
+			mt.opcacheResetScheduled = false
+			mt.opcacheResetMu.Unlock()
+		}()
+
+		if wait > 0 {
+			select {
+			case <-time.After(wait):
+			case <-mt.done:
+				// This instance is shutting down: give up rather than call
+				// rebootAllThreads on a stopped pool after an arbitrary delay.
+				return
+			}
+		}
+
+		// rebootAllThreads can no-op (already rebooting from an unrelated
+		// trigger, or the pool isn't Ready, e.g. mid-shutdown): only hold
+		// the cooldown, and only stamp lastOpcacheResetAt, when a reboot
+		// actually happened. Charging a cooldown against a reset that never
+		// occurred would silently swallow the next legitimate request too.
+		if !mt.rebootAllThreads() {
+			return
+		}
+
+		// Keep coalescing calls through a full cooldown measured from here,
+		// not just until the reboot itself finishes: threads can yield well
+		// within a grace period, so a call landing in the gap between
+		// "reboot done" and "cooldown elapsed" would otherwise open a brand
+		// new window instead of joining this one, turning one burst into
+		// two reboots.
+		select {
+		case <-time.After(opcacheResetCooldown):
+			mt.opcacheResetMu.Lock()
+			mt.lastOpcacheResetAt = time.Now()
+			mt.opcacheResetMu.Unlock()
+		case <-mt.done:
+		}
+	}()
 }
 
 func convertArgs(args []string) (C.int, []*C.char) {

frankenphp_test.go

@@ -184,6 +184,35 @@ func testFinishRequest(t *testing.T, opts *testOptions) {
 	}, opts)
 }
 
+// TestOpcacheResetIsThrottled reproduces https://github.com/php/frankenphp/issues/2553:
+// application code (e.g. some WordPress plugins) can call opcache_reset() on
+// every request, and each call reboots the whole PHP thread pool. Without a
+// cooldown, a burst of concurrent calls reboots the pool once per call,
+// pausing request handling for the whole site on every one of them.
+func TestOpcacheResetIsThrottled(t *testing.T) {
+	var buf fmt.Stringer
+	opts := &testOptions{nbParallelRequests: 1}
+	opts.logger, buf = newTestLogger(t)
+
+	runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) {
+		// Calls spaced out enough that, without a cooldown, each one lands
+		// after the previous reboot already finished (so each would trigger
+		// its own reboot), but still well within the cooldown window.
+		for n := 0; n < 10; n++ {
+			body, _ := testGet("http://example.com/opcache_reset.php", handler, t)
+			assert.Equal(t, "opcache reset done", body)
+			time.Sleep(50 * time.Millisecond)
+		}
+	}, opts)
+
+	require.Eventually(t, func() bool {
+		return strings.Contains(buf.String(), "thread reboot finished")
+	}, 2*time.Second, 10*time.Millisecond, "the throttled reboot should still eventually run")
+
+	count := strings.Count(buf.String(), "rebooting all PHP threads")
+	assert.Equal(t, 1, count, "opcache_reset() calls within the cooldown must be coalesced into a single reboot")
+}
+
 func TestServerVariable_module(t *testing.T) {
 	testServerVariable(t, nil)
 }

phpmainthread.go

@@ -26,6 +26,16 @@ type phpMainThread struct {
 	maxThreads  int
 	phpIni      map[string]string
 	isRebooting atomic.Bool
+
+	// throttles opcache_reset()-triggered reboots (see go_schedule_opcache_reset):
+	// application code can call opcache_reset() on every request, and each call
+	// would otherwise pause request handling for a full thread pool reboot.
+	// lastOpcacheResetAt is only ever set after a reboot actually happens, and
+	// opcacheResetScheduled coalesces (rather than drops) calls that arrive
+	// while one is already pending or in flight.
+	opcacheResetMu        sync.Mutex
+	lastOpcacheResetAt    time.Time
+	opcacheResetScheduled bool
 }
 
 var (
@@ -36,6 +46,9 @@ var (
 	// timeouts to wait for threads to yield before arming force-kill
 	shutDownGracePeriod = 30 * time.Second
 	rebootGracePeriod   = 6 * time.Second
+
+	// minimum interval between two opcache_reset()-triggered reboots
+	opcacheResetCooldown = 5 * time.Second
 )
 
 // initPHPThreads starts the main PHP thread,