Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
Commit Queue failed
The pull request was removed from the Commit Queue and labeled
commit-queue-failed
Full Commit Queue output |
|
This pull request has conflicts with its base branch, removing the |
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>
7047a9e to
bf07616
Compare
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
——
Ai generated, reviewed by me