feat: require_background_worker refinements

Collapses the three-mode require semantics into two, moves the
batch-declaration affordance to require instead of get_vars, and
renames require to ensure per @dunglas's concern that "require"
collides with PHP's language keyword (which takes a path).

- frankenphp_require_background_worker -> frankenphp_ensure_background_worker.
  "Ensure" captures the "make sure this is running, start it if it isn't"
  semantic without the keyword collision.

- Tolerant lazy-start inside frankenphp_handle_request: runtime ensure
  now behaves the same as non-worker mode (lazy-start + timeout +
  backoff tolerance). Bootstrap-before-handle_request keeps its
  fail-fast discipline. This lets processes start only the workers
  they actually exercise, instead of over-provisioning by pre-ensuring
  everything that might be needed.

- Multi-name ensure: frankenphp_ensure_background_worker now accepts
  string|array. Batch declaration with a shared deadline, fail-fast on
  any worker's boot failure in bootstrap mode. get_vars loses the array
  form, becoming single-name pure read.

- globalCtx.Done() wired into ensure's select cases, so in-flight
  calls unblock cleanly on FrankenPHP shutdown instead of waiting
  out their timeout.

- Fix: markBackgroundReady() is now called on every set_vars, not just
  the first. Previously sk.readyOnce gated it, leaving isBootingScript
  stuck at true after a crash-restart. That misclassified subsequent
  crashes as boot failures and kept the readyWorkers metric decremented.

Author: nicolas-grekas

background_worker.go

@@ -272,193 +272,156 @@ func getLookup(thread *phpThread) *backgroundWorkerLookup {
 	return nil
 }
 
-// requireMode describes how go_frankenphp_require_background_worker behaves
-// based on the caller context.
-type requireMode int
-
-const (
-	// requireModeBootstrap: HTTP worker before frankenphp_handle_request.
-	// Lazy-starts the worker; fail-fast on boot failure (no backoff tolerance).
-	requireModeBootstrap requireMode = iota
-	// requireModeRuntime: HTTP worker inside frankenphp_handle_request.
-	// Assert-only: worker must already be running; never lazy-starts.
-	requireModeRuntime
-	// requireModeNonWorker: non-worker context (classic request-per-process).
-	// Lazy-starts and tolerates transient boot failures via backoff.
-	requireModeNonWorker
-)
-
-// getRequireMode determines how require_background_worker should behave
-// based on the caller's thread handler state.
-func getRequireMode(thread *phpThread) requireMode {
-	if handler, ok := thread.handler.(*workerThread); ok {
-		if handler.isBootingScript {
-			return requireModeBootstrap
-		}
-		return requireModeRuntime
-	}
-	return requireModeNonWorker
+// isBootstrapEnsure reports whether the caller is an HTTP worker still
+// executing its bootstrap phase (before the first frankenphp_handle_request
+// call). Bootstrap is the only context that takes the strict fail-fast path;
+// everything else (worker runtime, classic request-per-process) uses the
+// tolerant lazy-start path.
+func isBootstrapEnsure(thread *phpThread) bool {
+	handler, ok := thread.handler.(*workerThread)
+	return ok && handler.isBootingScript
 }
 
-// go_frankenphp_require_background_worker declares a dependency on a background
-// worker. Semantics depend on the caller context:
+// go_frankenphp_ensure_background_worker declares a dependency on one or
+// more background workers. The worker is lazy-started if it isn't already
+// running, then the call blocks until it has called set_vars() at least once
+// (ready state) or the timeout expires.
 //
-//   - HTTP worker bootstrap (before frankenphp_handle_request): lazy-start and
-//     fail-fast on boot failure (throws as soon as a boot attempt fails).
-//   - HTTP worker runtime (inside frankenphp_handle_request): assert-only. The
-//     worker must already be running (either declared with num 1 in Caddyfile
-//     or previously required during bootstrap). Never lazy-starts.
-//   - Non-worker mode: lazy-start and tolerate transient boot failures (wait
-//     for the worker to recover up to the timeout).
+// When called from an HTTP worker's bootstrap (before frankenphp_handle_request),
+// it uses fail-fast semantics: any boot failure throws immediately with the
+// captured details, without waiting for the backoff/retry cycle. This turns
+// the bootstrap phase into a strict dependency declaration: a broken dep
+// visibly fails the HTTP worker rather than letting it serve degraded traffic.
 //
-//export go_frankenphp_require_background_worker
-func go_frankenphp_require_background_worker(threadIndex C.uintptr_t, name *C.char, nameLen C.size_t, timeoutMs C.int) *C.char {
+// Everywhere else (inside frankenphp_handle_request, classic request-per-process),
+// it waits tolerantly up to the timeout, letting the restart-with-backoff
+// cycle recover from transient boot failures.
+//
+//export go_frankenphp_ensure_background_worker
+func go_frankenphp_ensure_background_worker(threadIndex C.uintptr_t, names **C.char, nameLens *C.size_t, nameCount C.int, timeoutMs C.int) *C.char {
 	thread := phpThreads[threadIndex]
 	lookup := getLookup(thread)
 	if lookup == nil {
 		return C.CString("no background worker configured in this php_server")
 	}
 
-	goName := C.GoStringN(name, C.int(nameLen))
-	mode := getRequireMode(thread)
+	n := int(nameCount)
+	nameSlice := unsafe.Slice(names, n)
+	nameLenSlice := unsafe.Slice(nameLens, n)
+	bootstrap := isBootstrapEnsure(thread)
 
-	var sk *backgroundWorkerState
-	if mode == requireModeRuntime {
-		// Assert-only: worker must already be running.
-		registry := lookup.Resolve(goName)
-		if registry == nil {
-			return C.CString("background worker " + goName + " is not running; call frankenphp_require_background_worker before frankenphp_handle_request")
-		}
-		registry.mu.Lock()
-		sk = registry.workers[goName]
-		registry.mu.Unlock()
-		if sk == nil {
-			return C.CString("background worker " + goName + " is not running; call frankenphp_require_background_worker before frankenphp_handle_request")
-		}
-	} else {
-		// Bootstrap or non-worker: lazy-start if needed.
-		if err := startBackgroundWorker(thread, goName); err != nil {
+	sks := make([]*backgroundWorkerState, n)
+	goNames := make([]string, n)
+	for i := 0; i < n; i++ {
+		goNames[i] = C.GoStringN(nameSlice[i], C.int(nameLenSlice[i]))
+
+		if err := startBackgroundWorker(thread, goNames[i]); err != nil {
 			return C.CString(err.Error())
 		}
-		registry := lookup.Resolve(goName)
+		registry := lookup.Resolve(goNames[i])
 		if registry == nil {
-			return C.CString("background worker not found: " + goName)
+			return C.CString("background worker not found: " + goNames[i])
 		}
 		registry.mu.Lock()
-		sk = registry.workers[goName]
+		sks[i] = registry.workers[goNames[i]]
 		registry.mu.Unlock()
-		if sk == nil {
-			return C.CString("background worker not found: " + goName)
+		if sks[i] == nil {
+			return C.CString("background worker not found: " + goNames[i])
 		}
 	}
 
+	// Shared deadline across all workers.
 	deadline := time.After(time.Duration(timeoutMs) * time.Millisecond)
-	if mode == requireModeBootstrap {
-		// Fail-fast: watch for bootFailure alongside ready/deadline.
+	if bootstrap {
+		// Fail-fast: watch for bootFailure on any worker alongside ready/deadline.
 		ticker := time.NewTicker(50 * time.Millisecond)
 		defer ticker.Stop()
-		for {
-			select {
-			case <-sk.ready:
-				return nil
-			case <-deadline:
-				return C.CString(formatBackgroundWorkerTimeoutError(goName, sk))
-			case <-ticker.C:
-				if sk.bootFailure.Load() != nil {
-					return C.CString(formatBackgroundWorkerTimeoutError(goName, sk))
+		for i, sk := range sks {
+		wait:
+			for {
+				select {
+				case <-sk.ready:
+					break wait
+				case <-deadline:
+					return C.CString(formatBackgroundWorkerTimeoutError(goNames[i], sk))
+				case <-globalCtx.Done():
+					return C.CString("frankenphp is shutting down")
+				case <-ticker.C:
+					if sk.bootFailure.Load() != nil {
+						return C.CString(formatBackgroundWorkerTimeoutError(goNames[i], sk))
+					}
 				}
 			}
 		}
+		return nil
 	}
 
-	// Runtime or non-worker: wait for ready or timeout.
-	select {
-	case <-sk.ready:
-		return nil
-	case <-deadline:
-		return C.CString(formatBackgroundWorkerTimeoutError(goName, sk))
+	// Tolerant: wait for each worker's ready, shared deadline.
+	for i, sk := range sks {
+		select {
+		case <-sk.ready:
+			// ready
+		case <-deadline:
+			return C.CString(formatBackgroundWorkerTimeoutError(goNames[i], sk))
+		case <-globalCtx.Done():
+			return C.CString("frankenphp is shutting down")
+		}
 	}
+	return nil
 }
 
-// go_frankenphp_get_vars is a pure read: looks up already-running workers
-// and copies their vars into the return value. It never starts a worker or
-// waits for one to become ready. Callers should have ensured workers are
-// running via frankenphp_require_background_worker first.
+// go_frankenphp_get_vars is a pure read: looks up an already-running worker
+// and copies its vars into the return value. It never starts a worker or
+// waits for one to become ready. Callers should have ensured the worker is
+// running via frankenphp_ensure_background_worker first.
 //
-// callerVersions/outVersions: if callerVersions is non-nil and all versions match,
-// the copy is skipped entirely. outVersions receives current versions.
+// callerVersion/outVersion: if callerVersion is non-nil and the version matches,
+// the copy is skipped entirely. outVersion receives the current version.
 //
 //export go_frankenphp_get_vars
-func go_frankenphp_get_vars(threadIndex C.uintptr_t, names **C.char, nameLens *C.size_t, nameCount C.int, returnValue *C.zval, callerVersions *C.uint64_t, outVersions *C.uint64_t) *C.char {
+func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.size_t, returnValue *C.zval, callerVersion *C.uint64_t, outVersion *C.uint64_t) *C.char {
 	thread := phpThreads[threadIndex]
 	lookup := getLookup(thread)
 	if lookup == nil {
 		return C.CString("no background worker configured in this php_server")
 	}
 
-	n := int(nameCount)
-	nameSlice := unsafe.Slice(names, n)
-	nameLenSlice := unsafe.Slice(nameLens, n)
-
-	sks := make([]*backgroundWorkerState, n)
-	for i := 0; i < n; i++ {
-		goName := C.GoStringN(nameSlice[i], C.int(nameLenSlice[i]))
-
-		registry := lookup.Resolve(goName)
-		if registry == nil {
-			return C.CString("background worker not running: " + goName + " (call frankenphp_require_background_worker first)")
-		}
-		registry.mu.Lock()
-		sks[i] = registry.workers[goName]
-		registry.mu.Unlock()
-		if sks[i] == nil {
-			return C.CString("background worker not running: " + goName + " (call frankenphp_require_background_worker first)")
-		}
+	goName := C.GoStringN(name, C.int(nameLen))
 
-		// Must have reached ready state (i.e. called set_vars at least once).
-		select {
-		case <-sks[i].ready:
-		default:
-			return C.CString("background worker not ready: " + goName + " (no set_vars call yet)")
-		}
+	registry := lookup.Resolve(goName)
+	if registry == nil {
+		return C.CString("background worker not running: " + goName + " (call frankenphp_ensure_background_worker first)")
 	}
-
-	// Fast path: if all caller versions match, skip lock + copy entirely.
-	if callerVersions != nil && outVersions != nil {
-		callerVSlice := unsafe.Slice(callerVersions, n)
-		outVSlice := unsafe.Slice(outVersions, n)
-		allMatch := true
-		for i, sk := range sks {
-			v := sk.varsVersion.Load()
-			outVSlice[i] = C.uint64_t(v)
-			if uint64(callerVSlice[i]) != v {
-				allMatch = false
-			}
-		}
-		if allMatch {
-			return nil
-		}
+	registry.mu.Lock()
+	sk := registry.workers[goName]
+	registry.mu.Unlock()
+	if sk == nil {
+		return C.CString("background worker not running: " + goName + " (call frankenphp_ensure_background_worker first)")
 	}
 
-	// Take all read locks, collect pointers, copy via C helper, then release
-	ptrs := make([]unsafe.Pointer, n)
-	for i, sk := range sks {
-		sk.mu.RLock()
-		ptrs[i] = sk.varsPtr
+	// Must have reached ready state (i.e. called set_vars at least once).
+	select {
+	case <-sk.ready:
+	default:
+		return C.CString("background worker not ready: " + goName + " (no set_vars call yet)")
 	}
 
-	C.frankenphp_worker_copy_vars(returnValue, C.int(n), names, nameLens, (*unsafe.Pointer)(unsafe.Pointer(&ptrs[0])))
-
-	if outVersions != nil {
-		outVSlice := unsafe.Slice(outVersions, n)
-		for i, sk := range sks {
-			outVSlice[i] = C.uint64_t(sk.varsVersion.Load())
+	// Fast path: caller-version match skips the copy entirely.
+	if callerVersion != nil && outVersion != nil {
+		v := sk.varsVersion.Load()
+		*outVersion = C.uint64_t(v)
+		if uint64(*callerVersion) == v {
+			return nil
 		}
 	}
 
-	for _, sk := range sks {
-		sk.mu.RUnlock()
+	sk.mu.RLock()
+	ptr := sk.varsPtr
+	C.frankenphp_worker_copy_vars(returnValue, 1, &name, &nameLen, (*unsafe.Pointer)(unsafe.Pointer(&ptr)))
+	if outVersion != nil {
+		*outVersion = C.uint64_t(sk.varsVersion.Load())
 	}
+	sk.mu.RUnlock()
 
 	return nil
 }
@@ -498,10 +461,15 @@ func go_frankenphp_set_vars(threadIndex C.uintptr_t, varsPtr unsafe.Pointer, old
 	sk.varsVersion.Add(1)
 	sk.mu.Unlock()
 
+	// Close the ready channel at-most-once; a second close would panic.
 	sk.readyOnce.Do(func() {
-		bgHandler.markBackgroundReady()
 		close(sk.ready)
 	})
+	// markBackgroundReady is idempotent (guarded by isBootingScript). It must
+	// run on every set_vars because setupScript sets isBootingScript back to
+	// true on each (re)boot, and the next set_vars has to flip it to false
+	// again so subsequent crashes are classified as crashes, not boot failures.
+	bgHandler.markBackgroundReady()
 
 	return nil
 }

background_worker_test.go

@@ -94,6 +94,54 @@ func TestBackgroundWorkerSetVarsMarksWorkerReady(t *testing.T) {
 	assert.Equal(t, 1, testMetrics.readyCalls)
 }
 
+func TestBackgroundWorkerRestartReReachesReady(t *testing.T) {
+	// After a crash-restart cycle, setupScript flips isBootingScript back to
+	// true and the next set_vars must flip it back to false. Otherwise any
+	// subsequent crash would be misclassified as a boot failure and the
+	// readyWorkers metric would stay decremented.
+	originalMetrics := metrics
+	testMetrics := &backgroundWorkerTestMetrics{}
+	metrics = testMetrics
+	t.Cleanup(func() {
+		metrics = originalMetrics
+	})
+
+	handler := &backgroundWorkerThread{
+		thread: newPHPThread(0),
+		worker: &worker{
+			name:                   "background-worker",
+			fileName:               "background-worker.php",
+			maxConsecutiveFailures: -1,
+			backgroundWorker:       &backgroundWorkerState{ready: make(chan struct{})},
+		},
+		isBootingScript: true,
+	}
+
+	// First boot reaches ready.
+	handler.markBackgroundReady()
+	assert.False(t, handler.isBootingScript)
+	assert.Equal(t, 1, testMetrics.readyCalls)
+
+	// Worker crashes after ready: exit status != 0, isBootingScript == false.
+	handler.afterScriptExecution(1)
+	require.Len(t, testMetrics.stopCalls, 1)
+	assert.Equal(t, StopReason(StopReasonCrash), testMetrics.stopCalls[0])
+
+	// Restart: setupScript would set isBootingScript = true again.
+	handler.isBootingScript = true
+
+	// Next set_vars call must flip it back to false and increment ReadyWorker.
+	handler.markBackgroundReady()
+	assert.False(t, handler.isBootingScript)
+	assert.Equal(t, 2, testMetrics.readyCalls)
+
+	// A second crash at this point must still be classified as Crash, not BootFailure.
+	testMetrics.stopCalls = nil
+	handler.afterScriptExecution(1)
+	require.Len(t, testMetrics.stopCalls, 1)
+	assert.Equal(t, StopReason(StopReasonCrash), testMetrics.stopCalls[0])
+}
+
 func TestBackgroundWorkerBootFailureStaysBootFailureUntilReady(t *testing.T) {
 	originalMetrics := metrics
 	testMetrics := &backgroundWorkerTestMetrics{}

cli_test.go

@@ -42,7 +42,7 @@ func TestBackgroundWorkerFunctionsHiddenInCLI(t *testing.T) {
 	assert.NoError(t, err)
 
 	out := string(stdoutStderr)
-	assert.Contains(t, out, "frankenphp_require_background_worker:hidden")
+	assert.Contains(t, out, "frankenphp_ensure_background_worker:hidden")
 	assert.Contains(t, out, "frankenphp_set_vars:hidden")
 	assert.Contains(t, out, "frankenphp_get_vars:hidden")
 	assert.Contains(t, out, "frankenphp_get_worker_handle:hidden")