Address multiple TS-streams defects / clarifications - #7183
Open
jasnell wants to merge 9 commits into
Open
Conversation
… pattern The tee tests assumed tee-driven pulls always carry a byobRequest — an artifact of the original default-allocation behavior. The shared-queue tee model's null byobRequest is an accepted divergence: sources must check c.byobRequest and fall back to enqueue(). With the supported pattern the tee shapes are parity across implementations except the already-tracked readAtLeast tail-shape flag. Also records the resolved contract decisions in the transform ledger (terminate-after-cancel: the TypeScript behavior is the endorsed contract).
The queued byte cursor left BYOB descriptors pending forever when
close() arrived before they were satisfied: a plain read(view) parked at
close never resolved, and a read(view, {min}) holding below-minimum
bytes hung while reader.closed fulfilled. Descriptors facing the close
sentinel now settle through a deferred (one-microtask) end-of-data
commit implementing the decided C++ tail contract: element-aligned
partial fills resolve {done: false, value: partial} with the next read
observing EOF, and unfilled descriptors resolve done with an empty view.
The deferral is load-bearing — a source that calls respond(0) in the
same turn as close() still commits first via the spec's
RespondInClosedState fold shape, keeping the WPT read-min file green.
The native conduit's min-read under-delivery commit switches from the
fused {done: true, value: partial} EOF to the same split tail shape;
the C++ ReadableStreamNativeSource respond+close behavior is unchanged.
Pins flipped to parity: readable-byte closeBelowMin,
closeWithPendingUnfilledByobRead, readAtLeastByobReader; r2-patterns
byobReadAtLeastAutomatic, closedByobTeeOnStart,
identityTransformStreamReadAtLeast, partiallyFilledByobAtLeast, and the
tee complex r3 done flags; ts-webstreams
nativeBackedMinReadUnderDelivery.
The draining reader's empty-fallback read submitted a plain default read on the consumer, bypassing the auto-allocate descriptor synthesis the default reader performs — so the body and pipe pumps drove pull() with byobRequest null even when the source declared autoAllocateChunkSize, and respond()-driven sources needed a dual code path to work as bodies. The wait-read now goes through the shared descriptor synthesis when autoAllocateChunkSize is set, matching the C++ BYOB pump: pump pulls carry a byobRequest over the auto-allocated buffer, respond() commits zero-copy, and enqueue() still fulfills the read directly. Pins flipped to parity: readable-byte bodyPumpByobRequestPresence and drainingReaderDrivesBytePull. The respond.js body sources that declare autoAllocateChunkSize drop their enqueue fallback; sources without it (and tee sources, whose pulls still present null under the shared-queue model) stay dual-path.
…eviation The cancelWriteOnReleaseLock pin was read as a TypeScript orphan defect (queued write left pending when the writer releases). The spec says otherwise: WritableStreamDefaultWriterRelease rejects only the ready and closed promises, and queued writes stay in [[writeRequests]] to drain on the sink's schedule — the WPT piping/flow-control write-then-release- then-pipe tests require them to survive and complete under a relocking writer, and the TypeScript implementation passes those. The C++ cancel-queued-writes-at-release behavior is the deviation (the source of its flow-control expectedFailures). Ledger row 13 records the corrected reading; the pin keeps both sides asserted.
…eading The relockRespondOverflowSecondView pin framed the TypeScript behavior as accepting an oversized respond and fulfilling the second read untouched. The spec bounds-checks respond() against the HEAD descriptor (the released 4-byte one), enqueues its filled bytes, and serves the second read from the queue — and the TypeScript implementation does exactly that (probe-verified: 2 of the 3 responded bytes delivered, the third queued for the next read; the old pin's "untouched zeros" were the zeros the source wrote). C++'s RangeError, validated against the second read's smaller view, is the deviation. The pin now asserts the spec data flow on the TypeScript side; ledger row 10 records the corrected reading.
…ding The closedSourceToClosedDest pin annotated the C++ TypeError as the spec behavior. The spec's shutdown conditions apply in order — closing forward (source closed) precedes closing backward (dest closed), and CloseWithErrorPropagation resolves trivially against an already-closed destination — so the pipe fulfills, which is what the TypeScript implementation does and what WPT multiple-propagation's closed-to-closed test requires. The C++ dest-closed TypeError is the deviation. Comment and ledger row 10 corrected; assertions unchanged.
The identity and compression transforms errored the whole stream when an invalid chunk's write rejected — a consequence of routing validation errors through the standard sink machinery, where every rejection errors the stream. The decided contract (2026-08-28, matching the C++ internal controllers) is a per-write error: the offending write rejects, the stream stays usable, and queued writes behind it still deliver. The writable machinery gains a module-private non-fatal write-rejection channel (internalsForPipe.nonFatalWriteRejection) that rejects only the in-flight write's request and keeps the queue advancing; the identity and compression sinks route validation errors through it. CompressionStream/DecompressionStream now also accept SharedArrayBuffer-backed chunks by copying the shared bytes (decided 2026-08-28, matching the identity streams and C++; the strict [AllowShared]-less BufferSource reading was considered and overridden). The WPT bad-chunks files are disabled for the TypeScript configuration — the per-write contract leaves the stream usable, so the files' "read should reject" assertions hang — mirroring the C++ configuration. The pipe pump observes non-fatally rejected writes (a state unreachable under pure WHATWG semantics, where sink rejections error the dest): the pipe fails with the write's reason, aborting the destination and cancelling the source per the prevent flags, including when the rejection lands while a clean source-done shutdown is waiting for write acknowledgment. This surfaces the previously-silent stall when a number chunk was piped into a native identity stream. Pins flipped: identity rejectsNumberChunk, invalidChunkAfterQueuedValidWrites, alreadyDetachedBufferAtWrite (aftermath); compression stringChunkDiverges (aftermath), sharedArrayBufferChunkAccepted, invalidChunkRejectsWriteOnly (renamed from their -Diverges names); piping sabViewThroughCompressionRoundTrip, pipeThroughJsToInternal.
…tation
The workerd TransformStream({ expectedLength }) extension declares the
total bytes the readable side will produce, letting the C++ bridge emit
a concrete Content-Length for bodies built from such transforms. The
TypeScript implementation did not consult the property, so those bodies
went out chunked. The constructor now normalizes transformer
.expectedLength (same validation as the byte-source extension) and
installs it on the readable's default controller, where the existing
getControllerExpectedLength chain and the draining reader's
expectedLength pass-through pick it up. Advertisement only — the
transform does not enforce the total. Pins flipped to parity:
transform transformExpectedLengthFetchBody / RequestBody (the issue
#5113 regression coverage).
The pipeThroughJsToInternalCloses pin treated the TypeScript .locked getter as racy after a completed pipeThrough and pinned only getWriter. The getter and getWriter share one predicate and can never disagree at an instant; the observed flakiness was an unsynchronized read during the pipe's spec-shaped finalize cascade, whose lock release is not ordered against the output's done delivery (pipeThrough discards the pipe promise). One macrotask after the output completes the state is deterministically settled — probe-verified across repeated runs. The pin now asserts the settled contract: locked === false and getWriter() succeeds.
Contributor
|
@jasnell Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
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
1 participant
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.
Fix multiple TS streams issues and clarifications following the test consolidation