fix: a thread that ended on its own can still be asked to shut down

internal/state/state.go

@@ -215,8 +215,9 @@ func (ts *ThreadState) RequestSafeStateChange(nextState State) bool {
 	}
 	ts.mu.Unlock()
 
-	// wait for the state to change to a stable state
-	ts.WaitFor(Ready, Inactive, Reserved)
+	// Done too: a thread that ends on its own goes ShuttingDown then Done without
+	// being stable again, and only Done means its C side is gone
+	ts.WaitFor(Ready, Inactive, Reserved, Done)
 
 	return ts.RequestSafeStateChange(nextState)
 }

internal/state/state_test.go

@@ -53,6 +53,25 @@ func TestWaitForStateWithTimeoutGivesUpAndDropsItsSubscriber(t *testing.T) {
 	})
 }
 
+func TestRequestSafeStateChangeRefusesAThreadThatEndedOnItsOwn(t *testing.T) {
+	synctest.Test(t, func(t *testing.T) {
+		threadState := &ThreadState{currentState: TransitionComplete}
+
+		refused := make(chan bool, 1)
+		go func() {
+			refused <- threadState.RequestSafeStateChange(ShuttingDown)
+		}()
+
+		// once the request is parked, the thread ends by itself without passing
+		// through a stable state, the way a worker that fails to boot does
+		synctest.Wait()
+		threadState.Set(ShuttingDown)
+		threadState.Set(Done)
+
+		assert.False(t, <-refused, "a thread that is already done cannot be asked to shut down")
+	})
+}
+
 func assertNumberOfSubscribers(t *testing.T, threadState *ThreadState, expected int) {
 	t.Helper()
 

testdata/worker-reset-then-fail.php

@@ -0,0 +1,7 @@
+<?php
+
+// asks for a reboot of every thread while booting, then fails before
+// reaching frankenphp_handle_request()
+opcache_reset();
+
+exit(1);

worker_internal_test.go

@@ -120,3 +120,22 @@ func TestInitJoinsAThreadStuckInStartupTeardown(t *testing.T) {
 	assert.Error(t, <-initDone, "a worker failing to boot must fail Init")
 	assert.True(t, failedThread.state.Is(state.Reserved), "the thread must have exited and been reclaimed before Init returned, got: "+failedThread.state.Name())
 }
+
+// a reboot that meets a booting worker waits for its thread to settle, and a
+// worker that then gives up only reaches Done: Init() must still return
+func TestInitReturnsWhenARebootWaitsOnAWorkerThatGivesUp(t *testing.T) {
+	initDone := make(chan error, 1)
+	go func() {
+		initDone <- Init(
+			WithWorkers("reset-then-fail", testDataPath+"/worker-reset-then-fail.php", 1, WithWorkerMaxFailures(2)),
+			WithNumThreads(1),
+		)
+	}()
+
+	select {
+	case err := <-initDone:
+		require.ErrorContains(t, err, "too many consecutive failures")
+	case <-time.After(30 * time.Second):
+		t.Fatal("Init() hung behind a reboot waiting for a worker thread that ended on its own")
+	}
+}