Skip to content

Commit 83176c2

Browse files
feat: per-php_server background worker scoping
Sixth step of the split suggested in #2287. Builds on step 5: - BackgroundScope opaque type (int under the hood; obtain values via NextBackgroundWorkerScope, a counter). Zero is the global/embed scope; each php_server block gets a distinct non-zero scope. - Per-scope lookups: - backgroundLookups map[BackgroundScope]*backgroundWorkerLookup replaces the single global backgroundLookup. - buildBackgroundWorkerLookups iterates the declared bg workers into their scope's lookup; each declaration still gets its own registry. - getLookup(thread) resolves the active scope from the calling thread: worker handler -> request context -> global (0). - Options to drive the scope: - frankenphp.WithWorkerBackgroundScope(scope) tags a declaration with a scope. - frankenphp.WithRequestBackgroundScope(scope) tags a request so ensure/get_vars from a regular (non-worker) request resolve to the right block's lookup. - Caddy wiring: FrankenPHPModule.Provision allocates one scope per module instance (idempotent across re-provisions) and threads it into both worker declarations and ServeHTTP. Two php_server blocks can now declare background workers with the same user-facing name without colliding. - Global workersByName collision dropped for bg workers: bg workers resolve through their scope's lookup, so the same PHP-visible name can appear in two scopes without tripping the duplicate check. ## Tests - TestBackgroundWorkerScopeIsolation declares two bg workers named "shared" in distinct scopes, publishes distinct markers from each, and reads them back via scope-tagged requests. Confirms lookups resolve independently. All step-4 and step-5 tests still pass. ## Deferred to step 7 - Pools (num > 1 per named worker, max_threads > 1 for named workers). - Multiple declarations sharing one entrypoint file in one scope.
1 parent ee16104 commit 83176c2

10 files changed

Lines changed: 310 additions & 38 deletions

background_worker.go

Lines changed: 147 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"log/slog"
99
"strings"
1010
"sync"
11+
"sync/atomic"
1112
"time"
1213
"unsafe"
1314
)
@@ -17,10 +18,25 @@ import (
1718
// number of distinct lazy-started instances from a single catch-all.
1819
const defaultMaxBackgroundWorkers = 16
1920

20-
// backgroundLookup is the registry that resolves a worker name to either
21-
// a named declaration or the catch-all. Single global scope in this step;
22-
// step 6 replaces this with a per-php_server scope map.
23-
var backgroundLookup *backgroundWorkerLookup
21+
// BackgroundScope identifies an isolation boundary for background workers.
22+
// Each php_server block uses a distinct scope so that two blocks can
23+
// declare workers with the same name without conflict. The zero value is
24+
// the global/embed scope (used when no per-block scope was assigned).
25+
// Representation is opaque; obtain values via NextBackgroundWorkerScope.
26+
type BackgroundScope int
27+
28+
var backgroundScopeCounter atomic.Uint64
29+
30+
// NextBackgroundWorkerScope returns a unique scope for background worker
31+
// isolation. Each php_server block should call this once during
32+
// provisioning.
33+
func NextBackgroundWorkerScope() BackgroundScope {
34+
return BackgroundScope(backgroundScopeCounter.Add(1))
35+
}
36+
37+
// backgroundLookups maps scopes to their background worker lookup.
38+
// Scope 0 is the global/embed scope; each php_server block gets its own.
39+
var backgroundLookups map[BackgroundScope]*backgroundWorkerLookup
2440

2541
// backgroundWorkerLookup maps worker names to their registry, with a
2642
// separate slot for the catch-all (name-less) declaration.
@@ -57,11 +73,22 @@ type backgroundWorkerRegistry struct {
5773
workers map[string]*backgroundWorkerState
5874

5975
// Template options preserved so lazy-started workers inherit the same
60-
// env/watch/failure policy as their eagerly-started siblings.
76+
// scope/env/watch/failure policy as their eagerly-started siblings.
77+
scope BackgroundScope
6178
env PreparedEnv
6279
watch []string
6380
maxConsecutiveFailures int
6481
requestOptions []RequestOption
82+
83+
// declaredWorker is the pre-existing *worker struct for a named
84+
// declaration (num=0 lazy or num>=1 eager). It lets the lazy-start
85+
// path reuse this worker instead of scanning the global
86+
// workersByName map, which is not scope-aware: scoped bg workers
87+
// with the same user-facing name would otherwise collide into a
88+
// single *worker and overwrite each other's live state pointers.
89+
// nil for catch-all registries (each lazy-started name gets a
90+
// fresh worker).
91+
declaredWorker *worker
6592
}
6693

6794
func newBackgroundWorkerRegistry(entrypoint string) *backgroundWorkerRegistry {
@@ -115,59 +142,131 @@ func (registry *backgroundWorkerRegistry) abortStart(name string, bgw *backgroun
115142
bgw.abort(err)
116143
}
117144

118-
// buildBackgroundWorkerLookup constructs the name->registry map + catch-all
119-
// slot from declared worker options. Each declaration gets its own registry
120-
// so shared-entrypoint declarations keep their own template options.
121-
func buildBackgroundWorkerLookup(workers []*worker, opts []workerOpt) *backgroundWorkerLookup {
122-
var lookup *backgroundWorkerLookup
145+
// buildBackgroundWorkerLookups constructs a scope->lookup map from declared
146+
// worker options. Each scope (php_server block, or 0 for global/embed)
147+
// gets its own lookup so workers declared with the same name in different
148+
// blocks don't collide. Each declaration gets its own registry so shared-
149+
// entrypoint declarations keep their own template options.
150+
func buildBackgroundWorkerLookups(workers []*worker, opts []workerOpt) map[BackgroundScope]*backgroundWorkerLookup {
151+
lookups := make(map[BackgroundScope]*backgroundWorkerLookup)
123152

124153
for i, o := range opts {
125154
if !o.isBackgroundWorker {
126155
continue
127156
}
128-
if lookup == nil {
157+
158+
scope := o.backgroundScope
159+
lookup, ok := lookups[scope]
160+
if !ok {
129161
lookup = newBackgroundWorkerLookup()
162+
lookups[scope] = lookup
130163
}
131164

132-
registry := newBackgroundWorkerRegistry(o.fileName)
165+
w := workers[i]
166+
// Use the worker's normalized filename (EvalSymlinks + FastAbs
167+
// from newWorker) instead of the raw o.fileName so lazy-start
168+
// from a catch-all resolves the same entrypoint even if cwd or
169+
// the symlink target changes after init.
170+
registry := newBackgroundWorkerRegistry(w.fileName)
171+
registry.scope = scope
133172
registry.env = o.env
134173
registry.watch = o.watch
135174
registry.maxConsecutiveFailures = o.maxConsecutiveFailures
136175
registry.requestOptions = o.requestOptions
137176

138-
w := workers[i]
177+
w.backgroundScope = scope
139178
phpName := strings.TrimPrefix(w.name, "m#")
140179
if phpName != "" && phpName != w.fileName {
141180
if o.num > 0 {
142181
registry.num = o.num
143182
}
144183
lookup.byName[phpName] = registry
184+
// Named declaration: remember the *worker so lazy-start can
185+
// reuse it instead of scanning workersByName.
186+
registry.declaredWorker = w
187+
188+
// Pre-reserve the live state for eager (num >= 1) named
189+
// declarations: the worker thread created by initWorkers
190+
// will reserve it in setupScript, but any ensure_background_worker
191+
// call from an HTTP worker bootstrap that races ahead of
192+
// setupScript would otherwise see a missing entry and
193+
// lazy-start a duplicate. Reserving here makes the race
194+
// impossible; setupScript picks up the existing state.
195+
if o.num > 0 {
196+
bgw := &backgroundWorkerState{
197+
ready: make(chan struct{}),
198+
aborted: make(chan struct{}),
199+
}
200+
registry.workers[phpName] = bgw
201+
w.backgroundWorker = bgw
202+
}
145203
} else {
146204
maxW := defaultMaxBackgroundWorkers
147205
if o.maxThreads > 1 {
148206
maxW = o.maxThreads
149207
}
150208
registry.maxWorkers = maxW
151209
lookup.catchAll = registry
210+
// Catch-all declarations are strictly lazy-started: each
211+
// ensure() with an unmatched name spawns its own threads on
212+
// demand. Force num to 0 so initWorkers does not create
213+
// eager placeholder threads that would call reserve() under
214+
// the catch-all's own filename and consume one of the cap
215+
// slots before any real lazy-start happens.
216+
w.num = 0
152217
}
153218

154219
w.backgroundRegistry = registry
155220
}
156221

157-
return lookup
222+
if len(lookups) == 0 {
223+
return nil
224+
}
225+
return lookups
226+
}
227+
228+
// getLookup returns the background-worker lookup for the given thread.
229+
// The scope is resolved from the thread's handler (for worker threads
230+
// inheriting their worker's scope) or from the request context (for
231+
// regular HTTP threads with WithRequestBackgroundScope).
232+
//
233+
// If the resolved scope has no workers declared (its lookup is nil), the
234+
// caller falls through to the global/embed scope (0) so that globally-
235+
// declared workers remain reachable from scoped requests. Scopes that
236+
// declared their own workers stay strictly isolated because their lookup
237+
// is non-nil.
238+
func getLookup(thread *phpThread) *backgroundWorkerLookup {
239+
if backgroundLookups == nil {
240+
return nil
241+
}
242+
var scope BackgroundScope
243+
if handler, ok := thread.handler.(*workerThread); ok {
244+
scope = handler.worker.backgroundScope
245+
} else if handler, ok := thread.handler.(*backgroundWorkerThread); ok {
246+
scope = handler.worker.backgroundScope
247+
} else if fc, ok := fromContext(thread.context()); ok {
248+
scope = fc.backgroundScope
249+
}
250+
if scope != 0 {
251+
if l := backgroundLookups[scope]; l != nil {
252+
return l
253+
}
254+
}
255+
return backgroundLookups[0]
158256
}
159257

160258
// startBackgroundWorker lazy-starts the named worker if it is not already
161259
// running. Safe to call concurrently; only the first caller actually
162260
// starts the worker, the rest observe the existing state.
163-
func startBackgroundWorker(bgWorkerName string) error {
261+
func startBackgroundWorker(thread *phpThread, bgWorkerName string) error {
164262
if bgWorkerName == "" {
165263
return fmt.Errorf("background worker name must not be empty")
166264
}
167-
if backgroundLookup == nil {
265+
lookup := getLookup(thread)
266+
if lookup == nil {
168267
return fmt.Errorf("no background worker configured")
169268
}
170-
registry := backgroundLookup.Resolve(bgWorkerName)
269+
registry := lookup.Resolve(bgWorkerName)
171270
if registry == nil || registry.entrypoint == "" {
172271
return fmt.Errorf("no background worker configured for name %q", bgWorkerName)
173272
}
@@ -185,13 +284,20 @@ func startBackgroundWorkerWithRegistry(registry *backgroundWorkerRegistry, bgWor
185284

186285
numThreads := registry.maxThreads()
187286

188-
// A num=0 named declaration already created a worker struct at init
189-
// time; reuse it instead of creating a duplicate. For catch-all
190-
// instances (different names, different worker structs), create fresh.
287+
// Named declarations (num=0 lazy or num>=1 eager) already have a
288+
// pre-existing *worker struct recorded on the registry. Reuse it so
289+
// lazy-start doesn't create a duplicate and - crucially for per-
290+
// php_server isolation - doesn't route through the global
291+
// workersByName map, which is scope-agnostic and would make two
292+
// scopes sharing a user-facing name collide into the same *worker.
293+
// Catch-all registries leave declaredWorker nil so each lazy-started
294+
// name gets a fresh worker struct of its own.
191295
var w *worker
192-
if existing := workersByName[bgWorkerName]; existing != nil && existing.isBackgroundWorker {
193-
w = existing
296+
freshWorker := false
297+
if registry.declaredWorker != nil {
298+
w = registry.declaredWorker
194299
} else {
300+
freshWorker = true
195301
// Clone env and slices: newWorker mutates env (writes
196302
// FRANKENPHP_WORKER) and appends to requestOptions, so sharing
197303
// these across lazy-started instances would race with HTTP
@@ -208,6 +314,7 @@ func startBackgroundWorkerWithRegistry(registry *backgroundWorkerRegistry, bgWor
208314
fileName: registry.entrypoint,
209315
num: numThreads,
210316
isBackgroundWorker: true,
317+
backgroundScope: registry.scope,
211318
env: env,
212319
watch: watch,
213320
maxConsecutiveFailures: registry.maxConsecutiveFailures,
@@ -224,6 +331,10 @@ func startBackgroundWorkerWithRegistry(registry *backgroundWorkerRegistry, bgWor
224331
w.isBackgroundWorker = true
225332
w.backgroundWorker = bgw
226333
w.backgroundRegistry = registry
334+
// Redundant with newWorker's backgroundScope opt for fresh workers,
335+
// but necessary for declared workers whose scope is set on the
336+
// registry rather than on the workerOpt struct.
337+
w.backgroundScope = registry.scope
227338

228339
for i := 0; i < numThreads; i++ {
229340
t := getInactivePHPThread()
@@ -240,12 +351,14 @@ func startBackgroundWorkerWithRegistry(registry *backgroundWorkerRegistry, bgWor
240351
slog.Int("attached", i))
241352
break
242353
}
243-
if i == 0 && workersByName[bgWorkerName] != w {
244-
// Freshly-created worker: register it and add to the global list.
354+
if i == 0 && freshWorker {
355+
// Freshly-created catch-all instance: add to the global list so
356+
// RestartWorkers/DrainWorkers iterate it. Intentionally NOT
357+
// registered in workersByName - bg workers are resolved per-
358+
// scope via backgroundLookups, not via the global name map.
245359
scalingMu.Lock()
246360
workers = append(workers, w)
247361
scalingMu.Unlock()
248-
workersByName[bgWorkerName] = w
249362
}
250363
convertToBackgroundWorkerThread(t, w)
251364
}
@@ -283,17 +396,17 @@ func isBootstrapEnsure(thread *phpThread) bool {
283396
//export go_frankenphp_ensure_background_worker
284397
func go_frankenphp_ensure_background_worker(threadIndex C.uintptr_t, name *C.char, nameLen C.size_t, timeoutMs C.int) *C.char {
285398
thread := phpThreads[threadIndex]
286-
if backgroundLookup == nil {
399+
lookup := getLookup(thread)
400+
if lookup == nil {
287401
return C.CString("no background worker configured")
288402
}
289403

290404
goName := C.GoStringN(name, C.int(nameLen))
291405
bootstrap := isBootstrapEnsure(thread)
292-
293-
if err := startBackgroundWorker(goName); err != nil {
406+
if err := startBackgroundWorker(thread, goName); err != nil {
294407
return C.CString(err.Error())
295408
}
296-
registry := backgroundLookup.Resolve(goName)
409+
registry := lookup.Resolve(goName)
297410
if registry == nil {
298411
return C.CString("background worker not found: " + goName)
299412
}
@@ -386,13 +499,15 @@ func go_frankenphp_set_vars(threadIndex C.uintptr_t, varsPtr unsafe.Pointer, old
386499
// ensure() first, this returns a "not ready" error.
387500
//
388501
//export go_frankenphp_get_vars
389-
func go_frankenphp_get_vars(name *C.char, nameLen C.size_t, returnValue *C.zval) *C.char {
390-
if backgroundLookup == nil {
502+
func go_frankenphp_get_vars(threadIndex C.uintptr_t, name *C.char, nameLen C.size_t, returnValue *C.zval) *C.char {
503+
thread := phpThreads[threadIndex]
504+
lookup := getLookup(thread)
505+
if lookup == nil {
391506
return C.CString("no background worker configured")
392507
}
393508

394509
goName := C.GoStringN(name, C.int(nameLen))
395-
registry := backgroundLookup.Resolve(goName)
510+
registry := lookup.Resolve(goName)
396511
if registry == nil {
397512
return C.CString("background worker not found: " + goName + " (call frankenphp_ensure_background_worker first)")
398513
}

background_worker_scope_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package frankenphp_test
2+
3+
import (
4+
"errors"
5+
"io"
6+
"net/http/httptest"
7+
"os"
8+
"testing"
9+
10+
"github.com/dunglas/frankenphp"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
// TestBackgroundWorkerScopeIsolation declares two bg workers with the
16+
// *same* name in two distinct scopes. Requests scoped to each block must
17+
// resolve to their own worker, proving per-php_server isolation works.
18+
func TestBackgroundWorkerScopeIsolation(t *testing.T) {
19+
cwd, _ := os.Getwd()
20+
testDataDir := cwd + "/testdata/"
21+
22+
scopeA := frankenphp.NextBackgroundWorkerScope()
23+
scopeB := frankenphp.NextBackgroundWorkerScope()
24+
25+
require.NoError(t, frankenphp.Init(
26+
frankenphp.WithWorkers("shared", testDataDir+"background-worker-scope-a.php", 1,
27+
frankenphp.WithWorkerBackground(),
28+
frankenphp.WithWorkerBackgroundScope(scopeA)),
29+
frankenphp.WithWorkers("shared", testDataDir+"background-worker-scope-b.php", 1,
30+
frankenphp.WithWorkerBackground(),
31+
frankenphp.WithWorkerBackgroundScope(scopeB)),
32+
frankenphp.WithNumThreads(4),
33+
))
34+
t.Cleanup(frankenphp.Shutdown)
35+
36+
read := func(scope frankenphp.BackgroundScope) string {
37+
req := httptest.NewRequest("GET", "http://example.com/background-worker-scope-reader.php", nil)
38+
fr, err := frankenphp.NewRequestWithContext(req,
39+
frankenphp.WithRequestDocumentRoot(testDataDir, false),
40+
frankenphp.WithRequestBackgroundScope(scope),
41+
)
42+
require.NoError(t, err)
43+
w := httptest.NewRecorder()
44+
if err := frankenphp.ServeHTTP(w, fr); err != nil && !errors.As(err, &frankenphp.ErrRejected{}) {
45+
t.Fatalf("serve: %v", err)
46+
}
47+
body, _ := io.ReadAll(w.Result().Body)
48+
return string(body)
49+
}
50+
51+
// Each scope's worker publishes its own marker under the same name
52+
// ("shared"). The reader script reads get_vars('shared'); scope
53+
// selection on the request determines which worker is resolved.
54+
bodyA := read(scopeA)
55+
bodyB := read(scopeB)
56+
57+
assert.Contains(t, bodyA, "scope=A", "scopeA request should resolve to worker-scope-a:\n"+bodyA)
58+
assert.Contains(t, bodyB, "scope=B", "scopeB request should resolve to worker-scope-b:\n"+bodyB)
59+
}

0 commit comments

Comments
 (0)