Repository navigation
Conversation
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 <hello@matteocollina.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #67054 +/- ##
==========================================
+ Coverage 90.47% 90.64% +0.17%
==========================================
Files 791 797 +6
Lines 277245 278296 +1051
Branches 53310 53515 +205
==========================================
+ Hits 250834 252273 +1439
+ Misses 16811 16400 -411
- Partials 9600 9623 +23
🚀 New features to boost your workflow:
|
aduh95
approved these changes
Oct 27, 2026
Collaborator
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
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.
The
pipeTo()pump re-enters from two places that are not promise reactions:parkOnReady, stream: cut promise churn in webstreams hot paths #65138), whichwritableStreamUpdateBackpressureinvokes synchronously from whatever code cleared the backpressure;So once the pipe resumes, the source's
pull()and the writes it issues observe that context instead of the one of thepipeTo()call, which is what the promise-based pump (v24.20 and earlier, v26.9 and earlier) gave them: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
AsyncContextFramewrappers put it behind one more inlining level and cost 3-4% onpipe-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 onmain. It checks only the pulls and writes the pump drives: theclose()/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
🤖 Generated with Claude Code
https://claude.ai/code/session_01Sfj4nCwaLmXB1NxA7sEJfi