feat(preemption): bound the preempt drain and diagnose a coroutine that never cooperates - #30
Merged
Merged
Conversation
S6 settled that a preempt-suspended fiber may never be released, and issue #18 is the consequence: the drain that keeps it alive can never finish for a coroutine with no cooperative point. Bounding that drain means deciding to stop while a fiber is still suspended inside the interrupt callback, so the question S7 answers is what endings the process has from there. Measured on 8.4, every mode in its own subprocess (raw/s7_php84.txt): - letting the script end, uninstalling the hook first, and exit(70) all reach the same "Throwing from FFI callbacks is not allowed" at 255. exit() is the load-bearing one: it still runs request shutdown, so the process ends on the engine's fatal rather than on the status it was given. - a signal to self ends the process where it stands, destroys no fiber, and keeps everything already written on both streams. - a shutdown function registered from inside a shutdown function does run, so the ending can be deferred until after every shutdown function the application registered. - the control run drains a 2M-iteration loop in six resumes, which is where the size of a drain budget comes from. The 8.5 column is empty rather than assumed: this session had a single vendor tree resolved by 8.4. VERDICTS.md says so and says how to fill it in. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA
…t cooperate
drainPreempted() resumed a preempt-suspended coroutine until it
returned or parked, forever. For while (true) { $x++; } that is
forever literally: main returns, Go semantics say discard, and the
process hangs at shutdown with nothing to read (#18).
The drain now spends a budget - 64 resumes per coroutine, one second
of wall clock per attempt, and at least one resume each whatever the
clock says. Both halves are needed: a resume is not itself
time-bounded (S4: one sort() over 4M ints defers the next slice by
two seconds), and a clock alone could expire on a loaded machine
before a coroutine was resumed at all.
Giving up is not letting go. The straggler moves to a set the
scheduler holds for the rest of the process, run() throws
UndrainableCoroutineException naming each coroutine, the resumes and
seconds it was given and the line that spawned it, and the preemptor
ends the process with posix_kill(self, SIGKILL) from a shutdown
function it registers during shutdown - so every other shutdown
function still runs first. S7 measured why that ending and no other:
leaving the fiber, uninstalling the hook first and exit() all end at
255 on the engine's uncatchable fatal.
A panic keeps precedence over the straggler at run()'s boundary - it
is the bug the program actually has - and the shutdown handler
reports the straggler on STDERR either way, which is also the path a
forked worker and an exit() from main take.
One rule falls out of it and is enforced in drainPreempted(): a drain
may only resume while the slice timer is live. A resume returns
because the next tick takes the CPU back, not because the coroutine
hands it over, so draining with the clock disarmed is the same
unbounded wait one step further along.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA
The behaviour is how a process ends, and the regression it guards against is a drain that never returns - so a test that ran the runaway coroutine itself would hang the suite instead of failing it. tests/Support/runawayCoroutine.php is that process, and superviseChildProcess() runs it with a deadline, reads both pipes and reports whether it was signalled, so a regression comes back as timedOut: true. Two behaviours, two files: that the run ends with a diagnosis naming the coroutine and its spawn site, and that the ending is the runtime killing the process rather than the engine destroying the fiber - the second asserts the absence of "Throwing from FFI callbacks is not allowed", that the child died by SIGKILL, and that a shutdown function registered before the runtime still ran. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA
AGENTS.md said the scheduler must hold every preempted fiber and drain it, which is still true, and left "drains it forever" as the answer for a coroutine that never cooperates, which is no longer the behaviour. Both files now say the same thing as the code: the drain has a budget, giving up is not letting go, the run raises UndrainableCoroutineException naming the spawn site, and the process is ended with SIGKILL rather than by the engine - with the reason exit() is not an alternative, since that is the natural thing to reach for. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #18, via Option 1 — bound the drain and report.
drainPreempted()resumed a preempt-suspended coroutine until it returned or parked — forever, forwhile (true) { $x++; }. Main returns, Go semantics say discard, and the process hung at shutdown with nothing to read.The budget
64 resumes per coroutine per attempt, 1.0 s of wall clock per attempt, and at least one resume per coroutine whatever the clock says. Both halves are load-bearing: a resume is not itself time-bounded (S4 — one
sort()over 4M ints defers the next slice ~2 s), and a clock alone could expire on a loaded machine before a coroutine was ever given a chance. The control run in the new S7 spike drains a 2M-iteration loop in six resumes, so 64 is an order of magnitude of headroom. Counts accumulate across attempts, and each attempt re-offers its budget to earlier stragglers.Giving up is not letting go
A straggler moves to a set the scheduler holds for the rest of the process — the drain stopping is a decision about how long to keep resuming, never a decision to release the fiber.
run()throwsUndrainableCoroutineExceptionnaming each coroutine, the effort it was given, and the line that spawned it (already captured for the deadlock dump — zero new capture cost). A panic keeps precedence atrun()'s boundary; the straggler is still reported at shutdown.The ending — measured, not guessed
New spike S7 (
spikes/s7_undrainable_fiber_exit.php, verdict inspikes/VERDICTS.md) measured every ending available with an undrainable fiber alive: letting the script end, uninstalling the hook first, andexit()all die at 255 onThrowing from FFI callbacks is not allowed—exit()included, because it still runs request shutdown, which is where the engine destroys the fiber. Holding the fiber in a static changes nothing. Only a self-directed signal ends the process without destroying it, keeping both output streams.So the preemptor's shutdown backstop writes the diagnosis to STDERR and sends
SIGKILLto itself — from a shutdown function registered during shutdown, so every application shutdown function still runs first (S7--late-shutdown-function). SIGKILL rather than a politer signal because a handleable signal can be handled, and this one may not be declined: the only alternative is the fatal above. Without ext-posix the drain falls back to unbounded-but-diagnosed — the invariant outranks the hang. Criterion 3 holds throughout: the S6 fatal path is never reached.One rule fell out and is now enforced: a drain may only resume while the slice timer is live — a resume returns because the next tick takes the CPU back, not because the coroutine hands it over. (The first working draft diagnosed correctly and then hung in the backstop for exactly this reason.)
A runaway now ends in ~1.3 s with the diagnosis on both streams instead of hanging forever; the exit status is 137 (by signal), with the STDERR diagnosis immediately preceding it.
Acceptance criteria from #18
testACoroutineThatNeverCooperatesEndsTheRunWithADiagnosis.phpt, supervising the runaway in a child process with a deadline so a regression fails rather than hangs CI.testAnUndrainableCoroutineIsNeverLeftForTheEngineToDestroy.phptasserts the absence of the FFI fatal, death by signal (not exit 255), the diagnosis on stderr, and that an earlier-registered application shutdown function still ran.Verification
PHP 8.4.19,
-d ffi.enable=1 -d opcache.jit=off: full suiteOK (100 tests, 100 assertions), PHPStan level max clean,cs:checkclean. PHP 8.5 is CI-only (local vendor resolves the 8.4 z-engine line); S7's 8.5 column is marked not-measured inVERDICTS.mdrather than assumed.Known trade-offs, stated rather than hidden: a coroutine needing >~0.64 s of CPU to reach its first safe point is reported alongside the truly hopeless (indistinguishable from here; the remedy is the same — give it a safe point); exit 137 is confusable with an OOM kill, mitigated by the immediately-preceding STDERR diagnosis; a runaway inside a worker surfaces at the parent as
WorkerCrashedException(killed by signal 9) with the child's diagnosis on shared stderr.🤖 Generated with Claude Code
https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA
Generated by Claude Code