Skip to content

Commit 7b72f71

Browse files
refactor: decouple backgroundWorkerThread from workerThread
backgroundWorkerThread no longer embeds workerThread. It owns its own lifecycle state (failureCount, isBootingScript, dummyContext) and inlines the teardown logic. This prevents HTTP worker changes from accidentally affecting background workers. Also moves backgroundWorkerState and markBackgroundReady to threadbackgroundworker.go where they belong.
1 parent fb23fa9 commit 7b72f71

4 files changed

Lines changed: 107 additions & 34 deletions

File tree

background_worker.go

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"log/slog"
99
"strings"
1010
"sync"
11-
"sync/atomic"
1211
"time"
1312
"unsafe"
1413
)
@@ -56,14 +55,6 @@ func (l *backgroundWorkerLookup) Resolve(name string) *backgroundWorkerRegistry
5655
return l.catchAll
5756
}
5857

59-
type backgroundWorkerState struct {
60-
varsPtr unsafe.Pointer // *C.HashTable, persistent, managed by C
61-
mu sync.RWMutex
62-
varsVersion atomic.Uint64 // incremented on each set_vars call
63-
ready chan struct{}
64-
readyOnce sync.Once
65-
}
66-
6758
type backgroundWorkerRegistry struct {
6859
entrypoint string
6960
num int // threads per background worker (0 = lazy-start with 1 thread)
@@ -393,9 +384,8 @@ func go_frankenphp_worker_set_vars(threadIndex C.uintptr_t, varsPtr unsafe.Point
393384
if !ok || bgHandler.worker.backgroundWorker == nil {
394385
return C.CString("frankenphp_worker_set_vars() can only be called from a background worker")
395386
}
396-
handler := &bgHandler.workerThread
397387

398-
sk := handler.worker.backgroundWorker
388+
sk := bgHandler.worker.backgroundWorker
399389

400390
sk.mu.Lock()
401391
*oldPtr = sk.varsPtr
@@ -404,7 +394,7 @@ func go_frankenphp_worker_set_vars(threadIndex C.uintptr_t, varsPtr unsafe.Point
404394
sk.mu.Unlock()
405395

406396
sk.readyOnce.Do(func() {
407-
handler.markBackgroundReady()
397+
bgHandler.markBackgroundReady()
408398
close(sk.ready)
409399
})
410400

background_worker_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ func TestBackgroundWorkerSetVarsMarksWorkerReady(t *testing.T) {
7979
metrics = originalMetrics
8080
})
8181

82-
handler := &workerThread{
82+
handler := &backgroundWorkerThread{
8383
thread: newPHPThread(0),
8484
worker: &worker{name: "background-worker", fileName: "background-worker.php", maxConsecutiveFailures: -1},
8585
isBootingScript: true,
@@ -101,7 +101,7 @@ func TestBackgroundWorkerBootFailureStaysBootFailureUntilReady(t *testing.T) {
101101
metrics = originalMetrics
102102
})
103103

104-
handler := &workerThread{
104+
handler := &backgroundWorkerThread{
105105
thread: newPHPThread(0),
106106
worker: &worker{
107107
name: "background-worker",
@@ -111,14 +111,14 @@ func TestBackgroundWorkerBootFailureStaysBootFailureUntilReady(t *testing.T) {
111111
isBootingScript: true,
112112
}
113113

114-
tearDownWorkerScript(handler, 1)
114+
handler.afterScriptExecution(1)
115115
require.Len(t, testMetrics.stopCalls, 1)
116116
assert.Equal(t, StopReason(StopReasonBootFailure), testMetrics.stopCalls[0])
117117

118118
testMetrics.stopCalls = nil
119119
handler.isBootingScript = true
120120
handler.markBackgroundReady()
121-
tearDownWorkerScript(handler, 1)
121+
handler.afterScriptExecution(1)
122122
require.Len(t, testMetrics.stopCalls, 1)
123123
assert.Equal(t, StopReason(StopReasonCrash), testMetrics.stopCalls[0])
124124
}

threadbackgroundworker.go

Lines changed: 101 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,45 @@ package frankenphp
44
import "C"
55
import (
66
"context"
7+
"fmt"
78
"log/slog"
89
"path/filepath"
910
"strings"
11+
"sync"
12+
"sync/atomic"
13+
"time"
14+
"unsafe"
1015

1116
"github.com/dunglas/frankenphp/internal/state"
1217
)
1318

19+
// backgroundWorkerState holds the shared state for a single background worker.
20+
// Accessed by the background worker thread (writes) and HTTP worker threads (reads).
21+
type backgroundWorkerState struct {
22+
varsPtr unsafe.Pointer // *C.HashTable, persistent, managed by C
23+
mu sync.RWMutex
24+
varsVersion atomic.Uint64 // incremented on each set_vars call
25+
ready chan struct{}
26+
readyOnce sync.Once
27+
}
28+
1429
// backgroundWorkerThread handles background worker scripts.
15-
// Embeds workerThread for shared functionality (failure counting, metrics,
16-
// dummy context management) and overrides lifecycle methods.
30+
// Decoupled from workerThread — owns its own lifecycle state.
1731
type backgroundWorkerThread struct {
18-
workerThread
32+
state *state.ThreadState
33+
thread *phpThread
34+
worker *worker
35+
dummyFrankenPHPContext *frankenPHPContext
36+
dummyContext context.Context
37+
isBootingScript bool
38+
failureCount int
1939
}
2040

2141
func convertToBackgroundWorkerThread(thread *phpThread, worker *worker) {
2242
handler := &backgroundWorkerThread{
23-
workerThread: workerThread{
24-
state: thread.state,
25-
thread: thread,
26-
worker: worker,
27-
},
43+
state: thread.state,
44+
thread: thread,
45+
worker: worker,
2846
}
2947
thread.setHandler(handler)
3048
worker.attachThread(thread)
@@ -34,6 +52,17 @@ func (handler *backgroundWorkerThread) name() string {
3452
return "Background Worker PHP Thread - " + handler.worker.fileName
3553
}
3654

55+
func (handler *backgroundWorkerThread) frankenPHPContext() *frankenPHPContext {
56+
return handler.dummyFrankenPHPContext
57+
}
58+
59+
func (handler *backgroundWorkerThread) context() context.Context {
60+
if handler.dummyContext != nil {
61+
return handler.dummyContext
62+
}
63+
return globalCtx
64+
}
65+
3766
func (handler *backgroundWorkerThread) drain() {
3867
if fd := handler.worker.backgroundStopFdWrite.Load(); fd >= 0 {
3968
C.frankenphp_worker_write_stop_fd(C.int(fd))
@@ -115,5 +144,68 @@ func (handler *backgroundWorkerThread) setupScript() {
115144

116145
func (handler *backgroundWorkerThread) afterScriptExecution(exitStatus int) {
117146
handler.worker.backgroundStopFdWrite.Store(-1)
118-
tearDownWorkerScript(&handler.workerThread, exitStatus)
147+
worker := handler.worker
148+
handler.dummyFrankenPHPContext = nil
149+
handler.dummyContext = nil
150+
151+
// on exit status 0 we just run the worker script again
152+
if exitStatus == 0 && !handler.isBootingScript {
153+
metrics.StopWorker(worker.name, StopReasonRestart)
154+
155+
if globalLogger.Enabled(globalCtx, slog.LevelDebug) {
156+
globalLogger.LogAttrs(globalCtx, slog.LevelDebug, "restarting", slog.String("worker", worker.name), slog.Int("thread", handler.thread.threadIndex), slog.Int("exit_status", exitStatus))
157+
}
158+
159+
return
160+
}
161+
162+
// worker has thrown a fatal error or has not reached set_vars
163+
if handler.isBootingScript {
164+
metrics.StopWorker(worker.name, StopReasonBootFailure)
165+
} else {
166+
metrics.StopWorker(worker.name, StopReasonCrash)
167+
}
168+
169+
if !handler.isBootingScript {
170+
if globalLogger.Enabled(globalCtx, slog.LevelDebug) {
171+
globalLogger.LogAttrs(globalCtx, slog.LevelDebug, "restarting", slog.String("worker", worker.name), slog.Int("thread", handler.thread.threadIndex), slog.Int("exit_status", exitStatus))
172+
}
173+
174+
return
175+
}
176+
177+
if worker.maxConsecutiveFailures >= 0 && startupFailChan != nil && !watcherIsEnabled && handler.failureCount >= worker.maxConsecutiveFailures {
178+
startupFailChan <- fmt.Errorf("too many consecutive failures: worker %s has not reached frankenphp_handle_request()", worker.fileName)
179+
handler.thread.state.Set(state.ShuttingDown)
180+
return
181+
}
182+
183+
if watcherIsEnabled {
184+
if globalLogger.Enabled(globalCtx, slog.LevelError) {
185+
globalLogger.LogAttrs(globalCtx, slog.LevelWarn, "(watcher enabled) worker script has not reached frankenphp_handle_request()", slog.String("worker", worker.name), slog.Int("thread", handler.thread.threadIndex))
186+
}
187+
} else {
188+
if globalLogger.Enabled(globalCtx, slog.LevelWarn) {
189+
globalLogger.LogAttrs(globalCtx, slog.LevelWarn, "worker script has failed on restart", slog.String("worker", worker.name), slog.Int("thread", handler.thread.threadIndex), slog.Int("failures", handler.failureCount))
190+
}
191+
}
192+
193+
backoffDuration := time.Duration(handler.failureCount*handler.failureCount*100) * time.Millisecond
194+
if backoffDuration > time.Second {
195+
backoffDuration = time.Second
196+
}
197+
handler.failureCount++
198+
time.Sleep(backoffDuration)
199+
}
200+
201+
// markBackgroundReady resets failure state when the background worker
202+
// successfully calls set_vars for the first time.
203+
func (handler *backgroundWorkerThread) markBackgroundReady() {
204+
if !handler.isBootingScript {
205+
return
206+
}
207+
208+
handler.failureCount = 0
209+
handler.isBootingScript = false
210+
metrics.ReadyWorker(handler.worker.name)
119211
}

threadworker.go

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,6 @@ func setupWorkerScript(handler *workerThread, worker *worker) {
128128

129129
}
130130

131-
func (handler *workerThread) markBackgroundReady() {
132-
if !handler.isBootingScript {
133-
return
134-
}
135-
136-
handler.failureCount = 0
137-
handler.isBootingScript = false
138-
metrics.ReadyWorker(handler.worker.name)
139-
}
140131

141132
func tearDownWorkerScript(handler *workerThread, exitStatus int) {
142133
worker := handler.worker

0 commit comments

Comments
 (0)