Skip to content

Lock a closed or errored native stream on tee() - #7504

Merged
jasnell merged 1 commit into
mainfrom
jasnell/ts-streams-native-tee-lock
Sep 25, 2026
Merged

jasnell merged 1 commit into
mainfrom
jasnell/ts-streams-native-tee-lock

Conversation

@jasnell

@jasnell jasnell commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

tee() of a closed or errored native-backed stream returned two branches
in the same state but left the original unlocked, so it could be teed
again. The comment claimed this mirrored the queued path, which locks.
It now acquires the internal reader as the other tee paths do.

@jasnell
jasnell requested review from guybedford and npaun September 24, 2026 18:28
@jasnell
jasnell requested review from a team as code owners September 24, 2026 18:28
@jasnell
jasnell added this pull request to stack #7505 September 24, 2026 18:28
@ask-bonk

ask-bonk Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

@guybedford guybedford left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Verified locally: the suite passes on both cells, and teeClosedNativeBodyLocksOriginal fails on the TS cell with the acquireReadableStreamDefaultReader line removed, so it's a real pin. Also probed the errored half (a SELF body that controller.error()s mid-stream, read to error, reader released, then tee()): both TS and C++ leave the original locked, a second tee() throws, and both branch reads reject. The internal reader on an errored stream is fine since initializeReadableStreamGenericReader marks the rejected closed promise handled before rejecting it.

Optional: the commit title covers closed and errored, but the test only pins the closed case. An /erroring SELF endpoint alongside /delayed in main.js would let the test loop over a third body and cover the errored path too.

Base automatically changed from jasnell/ts-streams-husk-state to main September 25, 2026 01:42
tee() of a closed or errored native-backed stream returned two branches
in the same state but left the original unlocked, so it could be teed
again. The comment claimed this mirrored the queued path, which locks.
It now acquires the internal reader as the other tee paths do.

Parity with C++, whose tee() locks the original in every state. Behind
the experimental typescript_implemented_streams flag.
@jasnell
jasnell force-pushed the jasnell/ts-streams-native-tee-lock branch from 39cb489 to b705473 Compare September 25, 2026 01:42
@jasnell
jasnell merged commit baf490a into main Sep 25, 2026
24 of 25 checks passed
@jasnell
jasnell deleted the jasnell/ts-streams-native-tee-lock branch September 25, 2026 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants