Skip to content

Release the transformer once a TransformStream finishes - #7510

Merged
jasnell merged 1 commit into
mainfrom
jasnell/ts-streams-transformer-retention
Sep 25, 2026
Merged

jasnell merged 1 commit into
mainfrom
jasnell/ts-streams-transformer-retention

Conversation

@jasnell

@jasnell jasnell commented Sep 24, 2026

Copy link
Copy Markdown
Collaborator

ClearAlgorithms cleared the cancel and flush algorithms but not the
transform algorithm, and all three closures were built in the
TransformStream constructor, so transformer lived in the constructor's
shared context. Every closure the constructor creates keeps that context
alive, including the interop error hooks the halves hold for good, so the
transformer stayed reachable for as long as the stream did.

The algorithms are now built in a module-level helper, which alone
captures the transformer, and the transform algorithm lives on the
instance beside the other two. ClearAlgorithms drops all three, as the
spec has it, and the transformer is collectable after close, abort,
cancel, terminate() or error(), with the stream still held. Node agrees;
C++ keeps it after close (transform ledger #17).

A write that reaches the sink after readable.cancel() cleared the
algorithms, before the cancel errors the writable, is left undefined by
the spec; like Node, it drops the chunk and fulfills. Behind the
experimental typescript_implemented_streams flag.

@jasnell
jasnell added this pull request to stack #7511 September 24, 2026 19:12
@jasnell
jasnell requested review from a team as code owners September 24, 2026 19:12
@ask-bonk

ask-bonk Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

LGTM

github run

@jasnell
jasnell requested review from guybedford and npaun September 24, 2026 20:15
Base automatically changed from jasnell/ts-streams-transform-cancel-outcome to main September 25, 2026 18:23
@jasnell
jasnell force-pushed the jasnell/ts-streams-transformer-retention branch from 3c2e663 to 31239cc Compare September 25, 2026 18:42

@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.

The retention fix is right: with the algorithm closures built in makeTransformerAlgorithms, nothing in the constructor's scope references transformer from an inner function (the start() call is direct), so the shared context the interop hooks pin no longer carries it. Verified the gc test collects on all five endings.

One behavioral note on the new dropped-write path (inline), plus two nits:

  • The description says "transform ledger #17"; the diff adds row #18.
  • Branch is a few commits behind main (rebases cleanly).

Comment thread src/per_isolate/webstreams/transform.ts Outdated
ClearAlgorithms cleared the cancel and flush algorithms but not the
transform algorithm, and all three closures were built in the
TransformStream constructor, so `transformer` lived in the constructor's
shared context. Every closure the constructor creates keeps that context
alive, including the interop error hooks the halves hold for good, so the
transformer stayed reachable for as long as the stream did.

The algorithms are now built in a module-level helper, which alone
captures the transformer, and the transform algorithm lives on the
instance beside the other two. ClearAlgorithms drops all three, as the
spec has it, and the transformer is collectable after close, abort,
cancel, terminate() or error(), with the stream still held. Node agrees;
C++ keeps it after close (transform ledger #18).

A write that reaches the sink after readable.cancel() cleared the
algorithms, before the cancel errors the writable, is left undefined by
the spec; like Node, it drops the chunk and fulfills. Behind the
experimental typescript_implemented_streams flag.
@jasnell
jasnell force-pushed the jasnell/ts-streams-transformer-retention branch from 31239cc to 1495a62 Compare September 25, 2026 20:25
@jasnell
jasnell merged commit 7b31a25 into main Sep 25, 2026
38 of 39 checks passed
@jasnell
jasnell deleted the jasnell/ts-streams-transformer-retention branch September 25, 2026 21:19
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