fix: race condition in auto-scaler during Caddy config reload (#2318)

Author: nicolas-grekas

scaling.go

@@ -40,14 +40,17 @@ func initAutoScaling(mainThread *phpMainThread) {
 		return
 	}
 
+	done := mainThread.done
+	mstate := mainThread.state
+
 	scalingMu.Lock()
 	scaleChan = make(chan *frankenPHPContext)
 	maxScaledThreads := mainThread.maxThreads - mainThread.numThreads
 	autoScaledThreads = make([]*phpThread, 0, maxScaledThreads)
 	scalingMu.Unlock()
 
-	go startUpscalingThreads(maxScaledThreads, scaleChan, mainThread.done)
-	go startDownScalingThreads(mainThread.done)
+	go startUpscalingThreads(maxScaledThreads, scaleChan, done, mstate)
+	go startDownScalingThreads(done)
 }
 
 func drainAutoScaling() {
@@ -81,16 +84,16 @@ func addWorkerThread(worker *worker) (*phpThread, error) {
 }
 
 // scaleWorkerThread adds a worker PHP thread automatically
-func scaleWorkerThread(worker *worker) {
+func scaleWorkerThread(worker *worker, done chan struct{}, mstate *state.ThreadState) {
 	// probe CPU usage before acquiring the lock (avoids holding lock during 120ms sleep)
-	if !cpu.ProbeCPUs(cpuProbeTime, maxCpuUsageForScaling, mainThread.done) {
+	if !cpu.ProbeCPUs(cpuProbeTime, maxCpuUsageForScaling, done) {
 		return
 	}
 
 	scalingMu.Lock()
 	defer scalingMu.Unlock()
 
-	if !mainThread.state.Is(state.Ready) {
+	if !mstate.Is(state.Ready) {
 		return
 	}
 
@@ -111,16 +114,16 @@ func scaleWorkerThread(worker *worker) {
 }
 
 // scaleRegularThread adds a regular PHP thread automatically
-func scaleRegularThread() {
+func scaleRegularThread(done chan struct{}, mstate *state.ThreadState) {
 	// probe CPU usage before acquiring the lock (avoids holding lock during 120ms sleep)
-	if !cpu.ProbeCPUs(cpuProbeTime, maxCpuUsageForScaling, mainThread.done) {
+	if !cpu.ProbeCPUs(cpuProbeTime, maxCpuUsageForScaling, done) {
 		return
 	}
 
 	scalingMu.Lock()
 	defer scalingMu.Unlock()
 
-	if !mainThread.state.Is(state.Ready) {
+	if !mstate.Is(state.Ready) {
 		return
 	}
 
@@ -140,7 +143,7 @@ func scaleRegularThread() {
 	}
 }
 
-func startUpscalingThreads(maxScaledThreads int, scale chan *frankenPHPContext, done chan struct{}) {
+func startUpscalingThreads(maxScaledThreads int, scale chan *frankenPHPContext, done chan struct{}, mstate *state.ThreadState) {
 	for {
 		scalingMu.Lock()
 		scaledThreadCount := len(autoScaledThreads)
@@ -171,7 +174,7 @@ func startUpscalingThreads(maxScaledThreads int, scale chan *frankenPHPContext,
 
 			// if the request has been stalled long enough, scale
 			if fc.worker == nil {
-				scaleRegularThread()
+				scaleRegularThread(done, mstate)
 				continue
 			}
 
@@ -184,7 +187,7 @@ func startUpscalingThreads(maxScaledThreads int, scale chan *frankenPHPContext,
 				continue
 			}
 
-			scaleWorkerThread(fc.worker)
+			scaleWorkerThread(fc.worker, done, mstate)
 		case <-done:
 			return
 		}

scaling_test.go

@@ -20,7 +20,7 @@ func TestScaleARegularThreadUpAndDown(t *testing.T) {
 	autoScaledThread := phpThreads[1]
 
 	// scale up
-	scaleRegularThread()
+	scaleRegularThread(mainThread.done, mainThread.state)
 	assert.Equal(t, state.Ready, autoScaledThread.state.Get())
 	assert.IsType(t, &regularThread{}, autoScaledThread.handler)
 
@@ -48,7 +48,7 @@ func TestScaleAWorkerThreadUpAndDown(t *testing.T) {
 	autoScaledThread := phpThreads[2]
 
 	// scale up
-	scaleWorkerThread(workersByPath[workerPath])
+	scaleWorkerThread(workersByPath[workerPath], mainThread.done, mainThread.state)
 	assert.Equal(t, state.Ready, autoScaledThread.state.Get())
 
 	// on down-scale, the thread will be marked as inactive
@@ -69,7 +69,7 @@ func TestMaxIdleTimePreventsEarlyDeactivation(t *testing.T) {
 	autoScaledThread := phpThreads[1]
 
 	// scale up
-	scaleRegularThread()
+	scaleRegularThread(mainThread.done, mainThread.state)
 	assert.Equal(t, state.Ready, autoScaledThread.state.Get())
 
 	// set wait time to 30 minutes (less than 1 hour max idle time)