fix(thread-channel): honour the cancellation token on send and recv - #228
Merged
Conversation
EdmondDantes
force-pushed
the
fix/thread-channel-refused-value
branch
from
August 13, 2026 16:44
044cc23 to
58aff00
Compare
EdmondDantes
force-pushed
the
fix/thread-channel-cancellation-token
branch
2 times, most recently
from
August 13, 2026 16:45
3de312a to
37f8bb9
Compare
EdmondDantes
force-pushed
the
fix/thread-channel-refused-value
branch
from
August 13, 2026 18:02
58aff00 to
6cd7ab3
Compare
EdmondDantes
force-pushed
the
fix/thread-channel-cancellation-token
branch
from
August 13, 2026 18:02
37f8bb9 to
4ab6e98
Compare
EdmondDantes
changed the base branch from
fix/thread-channel-refused-value
to
main
August 13, 2026 18:07
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.
Stacked on #227.
ThreadChannel::send()andThreadChannel::recv()both declare?Async\Completable $cancellationTokenand neither used it.recv()passedNULLintothread_channel_receive(), which has taken a cancellation event and handled it correctly all along;send()had no parameter to take one at all. So a parked call could be broken only byCoroutine::cancel(), and a bounded wait had to be hand-built from a cancel race — whileAsync\Channelhonours the same argument on the same signature.Measured before:
After, all three report
Async\OperationCanceledExceptionat 301 ms.What changed
thread_channel_send()becomes a wrapper overthread_channel_send_ex(channel, value, cancellation), which registers the event alongside the sender trigger. The vtable entry keeps its signature, so the pool's concurrency gate is untouched.A wake is attributed to the token by asking the token, not by inspecting the buffer. One freed slot wakes every parked sender and one sent value wakes every parked receiver (
fire_all_triggers), so on a channel with more than one parked caller the losers of that race see an unchanged buffer with the token unfired. Reading that as a cancellation returnsfalsewith nothing thrown, andRETURN_THROWS()then reports an exception that was never raised:ZEND_ASSERT(EG(exception))aborts a debug build and a release build returnsNULLfromsend()/recv(). The receive side had this branch already, dead —METHOD(recv)passedNULLand the pool's only other call iswait_only, which returns earlier — so wiring the token is what made it reachable. Both sides now require the event to be closed, and a wake that is neither the token nor an exception parks again.A token that closed while an earlier round was parked is caught before re-registering:
zend_async_resume_when()refuses a closed event and returns false, and suspending anyway would arm the channel trigger alone, turning a bounded wait into an unbounded one.The userland methods resolve the token first — one that has already fired ends the call before it waits — and translate the
falsereturn throughreport_cancellation(). A timeout token raises itsTimeoutExceptionintoEG(exception)rather than carrying it on the event, soasync_resolve_cancel_token()cannot find it; it is held aside and chained onto theOperationCanceledExceptionafterwards. MatchingChannelmatters here:Async\timeout()alone would surfaceTimeoutException, which extends\Exceptionrather thanAsyncCancellation, socatch (AsyncCancellation)would work around one channel class and not the other.Tests
tests/thread_channel/046-cancellation_token.phptcovers a cancelledrecv, a cancelledsendon a full channel, a token that never fires, and the chained previous exception.tests/thread_channel/047-cancellation_token_spurious_wake.phptcovers the herd: two senders parked on a full channel and two receivers parked on an empty one, each woken by a single freed slot or a single value. It fails on the base branch with the assertion above, in both directions.tests/thread_channel,tests/thread_pool,tests/channelandtests/thread— 278 tests, 0 failed (1 pre-existing warning:thread_pool/030carries an--XFAIL--section and passes). Fullext/async: 2109 tests, 3 failed, the same three pre-existingtests/curlandtests/iofailures as onmain.