From 531a5abce29fa8c34c169eb322be4824b90304c3 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 21 Sep 2026 12:58:14 +0200 Subject: [PATCH 1/4] fix: a thread that ended on its own can still be asked to shut down RequestSafeStateChange() waits for a stable state, Ready, Inactive or Reserved, before answering. A thread that ends by itself never reaches one again: it goes to ShuttingDown, then Done, and only shutdown() sets Reserved, which is the very call left waiting. Shutdown() then hangs for ever on that thread. A worker that gives up during its boot takes exactly that route, past max_consecutive_failures during startup, so a Shutdown() racing it deadlocks today. Waiting for the terminal states as well answers the request the way it should be answered, with a refusal, and shutdown() takes the path it already has for a thread that is done. --- internal/state/state.go | 5 +++-- internal/state/state_test.go | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/internal/state/state.go b/internal/state/state.go index 839c4b5fce..c34100ac89 100644 --- a/internal/state/state.go +++ b/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) + // or Done: a thread that ended 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) } diff --git a/internal/state/state_test.go b/internal/state/state_test.go index 767a27606e..26863a6d8d 100644 --- a/internal/state/state_test.go +++ b/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() From e6dc03cc1261ef6a80f345c14fc30a571eaaf482 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Wed, 30 Sep 2026 18:01:54 +0200 Subject: [PATCH 2/4] test: a reboot that meets a worker giving up at boot no longer hangs Init() --- testdata/worker-reset-then-fail.php | 7 +++++++ worker_internal_test.go | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 testdata/worker-reset-then-fail.php diff --git a/testdata/worker-reset-then-fail.php b/testdata/worker-reset-then-fail.php new file mode 100644 index 0000000000..a4cdfbf3bd --- /dev/null +++ b/testdata/worker-reset-then-fail.php @@ -0,0 +1,7 @@ + Date: Thu, 1 Oct 2026 12:04:50 +0200 Subject: [PATCH 3/4] chore: make the Done wait comment stand on its own --- internal/state/state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/state/state.go b/internal/state/state.go index c34100ac89..7ff88bd54e 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -215,7 +215,7 @@ func (ts *ThreadState) RequestSafeStateChange(nextState State) bool { } ts.mu.Unlock() - // or Done: a thread that ended on its own goes ShuttingDown then Done without + // 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) From ab72f25449041b5ef366bbb0908b3cf943149312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Thu, 1 Oct 2026 12:04:50 +0200 Subject: [PATCH 4/4] test: use testDataPath for the reset-then-fail worker --- worker_internal_test.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/worker_internal_test.go b/worker_internal_test.go index d6ef8282c5..99f96126a2 100644 --- a/worker_internal_test.go +++ b/worker_internal_test.go @@ -124,12 +124,10 @@ func TestInitJoinsAThreadStuckInStartupTeardown(t *testing.T) { // 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) { - cwd, _ := os.Getwd() - initDone := make(chan error, 1) go func() { initDone <- Init( - WithWorkers("reset-then-fail", cwd+"/testdata/worker-reset-then-fail.php", 1, WithWorkerMaxFailures(2)), + WithWorkers("reset-then-fail", testDataPath+"/worker-reset-then-fail.php", 1, WithWorkerMaxFailures(2)), WithNumThreads(1), ) }()