GeezerVison is showing github.com — stream: keep async context of the pipeTo pump by mcollina · Pull Request #67054 · nodejs/node
Conversation
mcollina
commented
Oct 27, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
The pipeTo() pump re-enters from two places that are not promise reactions:
- the destination's ready hook (
parkOnReady, stream: cut promise churn in webstreams hot paths #65138), which writableStreamUpdateBackpressure invokes synchronously from whatever code cleared the backpressure;
- the chunk steps of its read request, which run in the context of whoever enqueued the chunk.
So once the pipe resumes, the source's pull() and the writes it issues observe that context instead of the one of the pipeTo() call, which is what the promise-based pump (v24.20 and earlier, v26.9 and earlier) gave them:
const pending = [];
const rs = new ReadableStream({ pull(c) { console.log(als.getStore()); c.enqueue(1); } }, { highWaterMark: 0 });
const ws = new WritableStream({
write() { return new Promise((resolve) => pending.push(resolve)); },
}, { highWaterMark: 1 });
als.run('piper', () => rs.pipeTo(ws));
// Resolving the writes from another context leaks it into pull():
// logs undefined (the creator's context) instead of 'piper' after the first chunk.This captures the async context frame when the pipe starts and runs the pump in it when it is entered from another context. The frame check uses the intrinsics directly: the AsyncContextFrame wrappers put it behind one more inlining level and cost 3-4% on pipe-to.js highWaterMarkW=1.
The new test covers a sink whose writes resolve from another context, a push source under backpressure, pipeThrough() with pulls resolved from another context, and a chain into a slow sink. Expectations match v24.18.0 (with and without --no-async-context-frame) and v26.5.1; the first three cases fail on main. It checks only the pulls and writes the pump drives: the close()/flush() contexts come from the pipe's close watcher, which #67052 fixes separately, so this PR does not depend on it.
Benchmarks (pipe-to, pipe-through, lifecycle, adapters, tee; 20 runs) are within ±2% in both directions (pipe-through transform +2.2% *, pipe-to 4096/1024 -2.0% *** which measured -0.1% in the previous run of the same code).
The leak shipped with #65138 in v24.21.0 and v26.10.0, so this should be backported to both lines.
Refs: #66906
Refs: #65138
Refs: #67052
——
Ai generated, reviewed by me
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
codecov
Bot
commented
Oct 27, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.67%. Comparing base (ee982db) to head (bf07616).
⚠️ Report is 135 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #67054 +/- ##
==========================================
- Coverage 90.67% 90.67% -0.01%
==========================================
Files 797 797
Lines 278791 278819 +28
Branches 53616 53619 +3
==========================================
+ Hits 252807 252823 +16
- Misses 16334 16344 +10
- Partials 9650 9652 +2 ... and 38 files with indirect coverage changes
🚀 New features to boost your workflow:
- ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Commit Queue failed
⚠ This PR has conflicts that must be resolved
The pull request was removed from the Commit Queue and labeled
commit-queue-failed
PRs whose Commit Queue landing failed and need manual intervention before retrying.
. After resolving the failure, remove that label and add
commit-queue
PRs queued for automated landing through the Commit Queue.
to retry.
Full Commit Queue output
�[36m⠋�[39m Loading data for nodejs/node/pull/67054
�[36m⠋�[39m Loading data for nodejs/node/pull/67054
�[36m⠋�[39m Getting collaborator contacts from README of nodejs/node
�[36m⠋�[39m Getting PR from nodejs/node/pull/67054
�[36m⠋�[39m Getting reviews from nodejs/node/pull/67054
�[36m⠋�[39m Getting comments from nodejs/node/pull/67054
�[36m⠋�[39m Getting commits from nodejs/node/pull/67054
✔ Done loading data for nodejs/node/pull/67054
----------------------------------- PR info ------------------------------------
Title stream: keep async context of the pipeTo pump (#67054)
Author Matteo Collina <[email protected]> (@mcollina)
Branch mcollina:webstreams-pipeto-async-context -> nodejs:main
Labels author ready, needs-ci, commit-queue, web streams
Commits 1
- stream: keep async context of the pipeTo pump
Committers 1
- Matteo Collina <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/67054
Refs: https://github.com/nodejs/node/issues/66906
Refs: https://github.com/nodejs/node/pull/65138
Refs: https://github.com/nodejs/node/pull/67052
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/67054
Refs: https://github.com/nodejs/node/issues/66906
Refs: https://github.com/nodejs/node/pull/65138
Refs: https://github.com/nodejs/node/pull/67052
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Paolo Insogna <[email protected]>
--------------------------------------------------------------------------------
ℹ This PR was created on Tue, 27 Oct 2026 12:25:59 GMT
✔ Approvals: 2
✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/67054#pullrequestreview-5671219316
✔ - Paolo Insogna (@ShogunPanda) (TSC): https://github.com/nodejs/node/pull/67054#pullrequestreview-5681138663
⚠ This PR has conflicts that must be resolved
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2026-10-27T17:18:07Z: https://ci.nodejs.org/job/node-test-pull-request/78689/
�[36m⠙�[39m Querying data for job/node-test-pull-request/78689/
�[36m⠙�[39m Querying data for job/node-test-pull-request/78689/
�[36m⠙�[39m Querying API for job/node-test-pull-request/78689/
✔ Build data downloaded
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
View workflow run
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
This pull request has conflicts with its base branch, removing the author ready label.
Please rebase your branch onto the latest base branch, resolve the conflicts locally, and force-push.
Afterwards the pull request needs a fresh collaborator approval, and a collaborator will add the label back once it is author ready again.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The pipeTo() pump re-enters from the destination's ready hook, which
writableStreamUpdateBackpressure invokes synchronously, and from the
chunk steps of its read request. Both run in the async context of the
code that cleared the backpressure or enqueued the chunk, so the
source's pull() and the writes issued after a resumption observed that
context instead of the one of the pipeTo() call, as they did with the
promise-based pump. Capture the async context frame when the pipe starts
and run the pump in it.
Refs: nodejs#66906
Refs: nodejs#65138
Signed-off-by: Matteo Collina <[email protected]>
nodejs-github-bot
pushed a commit
that referenced
this pull request
Nov 2, 2026
Writable.fromWeb() and Duplex.fromWeb() dispatch writes to the
WritableStream directly and complete them through a shared write
tracker. A write issued before the WritableStream had started was
queued and processed from its start step, so the sink ran in the async
context of the code that created the WritableStream, and the tracker
invoked the Node.js write callback synchronously from whatever context
completed the sink write, so the writes and final() it released ran
there too. The promise-based adapter waited for writer.ready and
received the result through a reaction, both in the context of the
Node.js write.
Dispatch writes issued before the start from a writer.ready reaction,
failing them if the stream was closed or errored meanwhile, and deliver
the write callback in the context of the Node.js write, from a reaction
when it completes in another context.
Refs: #66906
Refs: #65548
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: #67055
Refs: #67052
Refs: #67054
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
|
|
}
|
|
|
|
|
|
// Settles the pending callbacks from the given context until `promise`
|
|
|
// settles.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we should be using .finally
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
aduh95
pushed a commit
that referenced
this pull request
Nov 11, 2026
Writable.fromWeb() and Duplex.fromWeb() dispatch writes to the
WritableStream directly and complete them through a shared write
tracker. A write issued before the WritableStream had started was
queued and processed from its start step, so the sink ran in the async
context of the code that created the WritableStream, and the tracker
invoked the Node.js write callback synchronously from whatever context
completed the sink write, so the writes and final() it released ran
there too. The promise-based adapter waited for writer.ready and
received the result through a reaction, both in the context of the
Node.js write.
Dispatch writes issued before the start from a writer.ready reaction,
failing them if the stream was closed or errored meanwhile, and deliver
the write callback in the context of the Node.js write, from a reaction
when it completes in another context.
Refs: #66906
Refs: #65548
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: #67055
Refs: #67052
Refs: #67054
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
aduh95
pushed a commit
that referenced
this pull request
Nov 12, 2026
Writable.fromWeb() and Duplex.fromWeb() dispatch writes to the
WritableStream directly and complete them through a shared write
tracker. A write issued before the WritableStream had started was
queued and processed from its start step, so the sink ran in the async
context of the code that created the WritableStream, and the tracker
invoked the Node.js write callback synchronously from whatever context
completed the sink write, so the writes and final() it released ran
there too. The promise-based adapter waited for writer.ready and
received the result through a reaction, both in the context of the
Node.js write.
Dispatch writes issued before the start from a writer.ready reaction,
failing them if the stream was closed or errored meanwhile, and deliver
the write callback in the context of the Node.js write, from a reaction
when it completes in another context.
Refs: #66906
Refs: #65548
Signed-off-by: Matteo Collina <[email protected]>
PR-URL: #67055
Refs: #67052
Refs: #67054
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewers
aduh95
aduh95 approved these changes
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
commit-queue-failed
PRs whose Commit Queue landing failed and need manual intervention before retrying.
needs-ci
PRs that need a full CI run.
web streams
Issues and PRs related to the Web Streams API.
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|