Repository navigation
Transform a parked TransformStream write after one backpressure wait - #7412
Conversation
|
LGTM |
The transform sink's write algorithm waited for the backpressure-change promise in a loop, re-checking the flag after each wake-up. The spec waits once: a read's pull releases the write, and an enqueue on a stored controller in that same turn re-asserts backpressure before the write's reaction runs, yet the write still transforms. Under the loop it parked again until the next read, so a producer that awaited each write before reading again deadlocked, and the transformer never saw the chunk. The C++ implementation waits once. Both sink writes now wait once and then check the writable for erroring, as before. The zero-algorithm path's loop could never iterate twice, since that readable has no controller to enqueue through; it changes for uniformity. Pinned in the transform suite's backpressure.js. The release through a pending read is parity. The release through a dequeue at readable hwm 1 is a divergence (ledger #14): the C++ readable does not pull after a dequeue that leaves room (readable ledger #5), so its write stays parked until a read finds the queue empty. An error() in the release's turn is a divergence too (ledger #15): the woken TypeScript write rejects with the writable's stored error, the C++ one has already lost its writable reference and algorithms and rejects with the identity path's enqueue TypeError.
1ec78d5 to
ae8979d
Compare
guybedford
left a comment
There was a problem hiding this comment.
LGTM. The while -> if is exactly the spec shape: TransformStreamDefaultSinkWriteAlgorithm step 3 reacts to [[backpressureChangePromise]] once, checks erroring, then performs the transform, and never re-reads [[backpressure]]. The deadlock analysis in the description checks out (a read's pull releases the write; a same-turn enqueue on a stored controller re-asserts before the reaction runs), and the elided-path claim holds since that readable's controller is not user-reachable.
Verified locally: with main's transform.ts swapped in, transform-ts@ fails on the two new same-turn tests, so the regression is caught; with the PR all 18 transform targets pass across the cpp / cpp-legacy / cpp-legacy-backpressure / cpp-pedantic / ts cells, and //src/wpt:streams-ts@ (which runs transform-streams/backpressure.any.js with no expected failures) and //src/tests/node/stream/... pass too.
Two minor, non-blocking notes inline.
The transform sink's write algorithm waited for the backpressure-change
promise in a loop, re-checking the flag after each wake-up. The spec
waits once: a read's pull releases the write, and an enqueue on a stored
controller in that same turn re-asserts backpressure before the write's
reaction runs, yet the write still transforms. Under the loop it parked
again until the next read, so a producer that awaited each write before
reading again deadlocked, and the transformer never saw the chunk. The
C++ implementation waits once.
Both sink writes now wait once and then check the writable for
erroring, as before. The zero-algorithm path's loop could never iterate
twice, since that readable has no controller to enqueue through; it
changes for uniformity.
Pinned in the transform suite's backpressure.js. The release through a
pending read is parity. The release through a dequeue at readable hwm 1
is a divergence (ledger #14): the C++ readable does not pull after a
dequeue that leaves room (readable ledger #5), so its write stays parked
until a read finds the queue empty. An error() in the release's turn is
a divergence too (ledger #15): the woken TypeScript write rejects with
the writable's stored error, the C++ one has already lost its writable
reference and algorithms and rejects with the identity path's enqueue
TypeError.