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

[nicolas-grekas]

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 the only code that sets Reserved is the shutdown() call left waiting. Shutdown() then hangs on that thread for ever.

A worker that gives up during its boot takes exactly that route, past max_consecutive_failures during startup. Shutdown() cannot meet it, Init() holds startupMu, but a reboot can: opcache_reset() from a booting worker script, or RestartWorkers() during initWorkers(). Waiting for Done as well answers the request with a refusal once the thread is gone, which is what shutdown() and forceReboot() expect from one. Done is the only state that means the C side has exited: ShuttingDown is set by the thread itself, with its TSRM teardown still ahead.

The test fails with a deadlock on main and passes with the fix.

[AlliBalliBaba]

Not sure this is a good idea. "Done" is kind of a transitionary state, meaning another thread probably is waiting for "Done" to set it to "Reserved".

Is this even necessary if #2662 is merged instead?

[nicolas-grekas]

Still needed with #2662: that one runs once RequestSafeStateChange() has returned, this one is what lets it return. A thread still booting (TransitionComplete) that then fails on its own goes ShuttingDown → Done and never reaches a state the old wait listens to, so the caller hangs before #2662's code runs. On main that's forceReboot() from RestartWorkers() or opcache_reset() during startup; on #2617 it's the boot timeout, which returns from initWorkers() while the thread is still booting.

On Done being transitional: the waiter only reads it and refuses, it never writes. Reserved was already in the set, so whichever of the two it sees the answer is the same, and whoever moves Done to Reserved isn't affected.

[nicolas-grekas]

e6dc03c adds a test that hangs on main with it in. A worker calls opcache_reset() while booting, then gives up. The reboot holds scalingMu and waits in forceReboot() for the thread, which ends on its own at Done, and Shutdown() queues behind scalingMu: Init() never returns. With this PR it returns the boot error in 0.1s.

On main alone that's a narrow race; the stronger driver is #2617, whose boot timeout returns from initWorkers() while the thread is still booting.

[dunglas]

Thanks @nicolas-grekas!