fix: join a thread whose shutdown was only requested

phpthread.go

@@ -114,36 +114,47 @@ func (thread *phpThread) forceReboot() bool {
 // shutdown the underlying PHP thread
 func (thread *phpThread) shutdown() {
 	if !thread.state.RequestSafeStateChange(state.ShuttingDown) {
-		// thread is already shutting down, prefer the stable reserved state over done
+		// a worker that failed to boot published ShuttingDown itself and is
+		// still walking its C exit path: join it before TSRM is torn down
+		thread.waitForExit(state.Done, state.Reserved)
+		// prefer the stable reserved state over done
 		_ = thread.state.CompareAndSwap(state.Done, state.Reserved)
+
 		return
 	}
 
 	close(thread.drainChan)
 
+	thread.waitForExit(state.Done)
+
+	thread.drainChan = make(chan struct{})
+
+	// threads go back to the reserved state from which they can be booted again
+	thread.state.Set(state.Reserved)
+}
+
+// waitForExit joins the C thread, arming force-kill past the grace period
+func (thread *phpThread) waitForExit(exitStates ...state.State) {
+	if thread.state.WaitForStateWithTimeout(shutDownGracePeriod, exitStates...) {
+		return
+	}
+
 	// Arm force-kill after the grace period to wake any thread stuck in
 	// a blocking syscall (sleep, blocking I/O). The wait remains
 	// unbounded - on platforms where force-kill cannot interrupt the
 	// syscall (macOS, Windows non-alertable Sleep) the thread will exit
 	// when the syscall completes naturally; the operator's orchestrator
 	// is responsible for any harder timeout.
-	if !thread.state.WaitForStateWithTimeout(shutDownGracePeriod, state.Done) {
-		globalLogger.LogAttrs(
-			globalCtx,
-			slog.LevelWarn,
-			"force-killing thread on shutdown timeout",
-			slog.String("name", thread.name()),
-			slog.String("state", thread.state.Name()),
-			slog.String("timeout", shutDownGracePeriod.String()),
-		)
-		thread.sendKillSignal()
-		thread.state.WaitFor(state.Done)
-	}
-
-	thread.drainChan = make(chan struct{})
-
-	// threads go back to the reserved state from which they can be booted again
-	thread.state.Set(state.Reserved)
+	globalLogger.LogAttrs(
+		globalCtx,
+		slog.LevelWarn,
+		"force-killing thread on shutdown timeout",
+		slog.String("name", thread.name()),
+		slog.String("state", thread.state.Name()),
+		slog.String("timeout", shutDownGracePeriod.String()),
+	)
+	thread.sendKillSignal()
+	thread.state.WaitFor(exitStates...)
 }
 
 // setHandler changes the thread handler safely

worker_internal_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/dunglas/frankenphp/internal/state"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -76,3 +77,46 @@ func TestRestartWorkersForceKillsStuckThread(t *testing.T) {
 	assert.NotContains(t, recorder.Body.String(), "should not reach",
 		"VM interrupt was never observed; sleep returned naturally")
 }
+
+// Init() must not return, nor the runtime tear SAPI/TSRM down, while a worker
+// that failed to boot is still inside its shutdown handler
+func TestInitJoinsAThreadStuckInStartupTeardown(t *testing.T) {
+	t.Cleanup(Shutdown)
+
+	var failedThread *phpThread
+	held := make(chan struct{})
+	release := make(chan struct{})
+	initDone := make(chan error, 1)
+
+	go func() {
+		initDone <- Init(
+			WithNumThreads(2),
+			WithWorkers("held-failing-worker", testDataPath+"/failing-worker.php", 1,
+				WithWorkerMaxFailures(0),
+				WithWorkerOnShutdown(func(threadIndex int) {
+					failedThread = phpThreads[threadIndex]
+					close(held)
+					<-release
+				}),
+			),
+		)
+	}()
+
+	select {
+	case <-held:
+	case err := <-initDone:
+		t.Fatalf("Init returned before the failed worker thread started its teardown: %v", err)
+	}
+
+	select {
+	case err := <-initDone:
+		t.Fatalf("Init returned while the failed worker thread was still shutting down (state: %s, error: %v)", failedThread.state.Name(), err)
+	case <-time.After(500 * time.Millisecond):
+	}
+
+	require.True(t, failedThread.state.Is(state.ShuttingDown), "thread should still be shutting down")
+	close(release)
+
+	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())
+}