Skip to content

Commit e65b707

Browse files
mcollinaaduh95
authored andcommitted
stream: keep async context of parked writes
A TransformStream write that parks on backpressure was resumed from the read that lifted the backpressure, so transform(), the writes queued behind it, and flush() ran in the reader's async context instead of the writer's. Capture the async context frame when the write parks and restore it while the continuation runs, matching the context the spec's [[backpressureChangePromise]] reaction had. Fixes: #66906 Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #66907 Backport-PR-URL: #66915 Fixes: #66906 Refs: #66906 Refs: #66907 Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 458a0a8 commit e65b707

3 files changed

Lines changed: 162 additions & 16 deletions

File tree

‎lib/internal/webstreams/transformstream.js‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ const {
2525
DOMException,
2626
} = internalBinding('messaging');
2727

28+
const AsyncContextFrame = require('internal/async_context_frame');
29+
const { enabledHooksExist } = require('internal/async_hooks');
30+
2831
const {
2932
customInspectSymbol: kInspect,
3033
kEmptyObject,
@@ -267,6 +270,8 @@ class TransformStreamState {
267270
pullPending = false;
268271
pendingWriteParked = false;
269272
pendingWriteChunk = undefined;
273+
pendingWriteContextFrame = undefined;
274+
pendingWriteResolve = undefined;
270275
writeContinuation = undefined;
271276
}
272277
ObjectSetPrototypeOf(TransformStreamState.prototype, null);
@@ -474,7 +479,13 @@ function transformStreamSetBackpressure(stream, backpressure) {
474479
state.readable[kState].controller[kState].pullFulfilled);
475480
}
476481
} else if (state.pendingWriteParked) {
477-
PromisePrototypeThen(kResolvedPromise, state.writeContinuation);
482+
const resolve = state.pendingWriteResolve;
483+
if (resolve !== undefined) {
484+
state.pendingWriteResolve = undefined;
485+
resolve();
486+
} else {
487+
PromisePrototypeThen(kResolvedPromise, state.writeContinuation);
488+
}
478489
}
479490
}
480491

@@ -626,31 +637,48 @@ function transformStreamDefaultSinkWriteAlgorithm(stream, chunk) {
626637
// them before invoking the write algorithm), replacing the promise
627638
// record the old code allocated and resolved per parked chunk. The
628639
// writable dispatches a single write at a time, so one pending slot
629-
// suffices.
640+
// suffices. The old reaction was registered here, so the continuation
641+
// runs in the async context captured at park time rather than in the
642+
// context of the read that lifts the backpressure.
630643
assert(!state.pendingWriteParked);
631644
state.pendingWriteParked = true;
632645
state.pendingWriteChunk = chunk;
646+
state.pendingWriteContextFrame = AsyncContextFrame.current();
633647
state.writeContinuation ??= () => {
634648
const pendingChunk = state.pendingWriteChunk;
649+
const prior = AsyncContextFrame.exchange(state.pendingWriteContextFrame);
635650
state.pendingWriteParked = false;
636651
state.pendingWriteChunk = undefined;
637-
const writableState = state.writable[kState];
638-
const writableControllerState = writableState.controller[kState];
639-
if (writableState.state === 'erroring') {
640-
const error = writableState.storedError;
652+
state.pendingWriteContextFrame = undefined;
653+
try {
654+
const writableState = state.writable[kState];
655+
const writableControllerState = writableState.controller[kState];
656+
if (writableState.state === 'erroring') {
657+
const error = writableState.storedError;
658+
PromisePrototypeThen(
659+
kResolvedPromise,
660+
() => writableControllerState.writeRejected(error));
661+
return;
662+
}
663+
assert(writableState.state === 'writable');
641664
PromisePrototypeThen(
642-
kResolvedPromise,
643-
() => writableControllerState.writeRejected(error));
644-
return;
665+
transformStreamDefaultControllerPerformTransform(
666+
controller,
667+
pendingChunk),
668+
writableControllerState.writeFulfilled,
669+
writableControllerState.writeRejected);
670+
} finally {
671+
AsyncContextFrame.set(prior);
645672
}
646-
assert(writableState.state === 'writable');
647-
PromisePrototypeThen(
648-
transformStreamDefaultControllerPerformTransform(
649-
controller,
650-
pendingChunk),
651-
writableControllerState.writeFulfilled,
652-
writableControllerState.writeRejected);
653673
};
674+
if (enabledHooksExist()) {
675+
// Async hooks (and the async_hooks-based AsyncLocalStorage) track
676+
// context through promise reactions, so register the continuation
677+
// now, in the writer's context, and resolve it on unpark.
678+
const { promise, resolve } = PromiseWithResolvers();
679+
state.pendingWriteResolve = resolve;
680+
PromisePrototypeThen(promise, state.writeContinuation);
681+
}
654682
return kParkedAlgorithmResult;
655683
}
656684
return transformStreamDefaultControllerPerformTransform(controller, chunk);
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Flags: --no-async-context-frame
2+
'use strict';
3+
4+
// A write parked on backpressure must run transform() and the writes queued
5+
// behind it in the async context of the writer, not in the context of the
6+
// read that lifts the backpressure.
7+
// This variant covers the async_hooks-based AsyncLocalStorage.
8+
// Refs: https://github.com/nodejs/node/issues/66906
9+
10+
const common = require('../common');
11+
const assert = require('assert');
12+
const { AsyncLocalStorage } = require('async_hooks');
13+
const { setImmediate } = require('timers/promises');
14+
15+
const als = new AsyncLocalStorage();
16+
17+
function createPipeline(seen) {
18+
return new ReadableStream({
19+
start(controller) {
20+
for (const chunk of 'abc') controller.enqueue(chunk);
21+
controller.close();
22+
},
23+
}).pipeThrough(new TransformStream({
24+
transform(chunk, controller) {
25+
seen.push(`${chunk}:${als.getStore() ?? 'none'}`);
26+
controller.enqueue(chunk);
27+
},
28+
flush() {
29+
seen.push(`flush:${als.getStore() ?? 'none'}`);
30+
},
31+
}));
32+
}
33+
34+
async function consume(readable) {
35+
// eslint-disable-next-line no-unused-vars
36+
for await (const _ of readable);
37+
}
38+
39+
(async () => {
40+
{
41+
const seen = [];
42+
const readable = als.run('writer', () => createPipeline(seen));
43+
// Let the pipe park its first write on backpressure.
44+
await setImmediate();
45+
await consume(readable);
46+
assert.deepStrictEqual(seen, [
47+
'a:writer', 'b:writer', 'c:writer', 'flush:writer',
48+
]);
49+
}
50+
51+
{
52+
const seen = [];
53+
const readable = createPipeline(seen);
54+
await setImmediate();
55+
await als.run('reader', () => consume(readable));
56+
assert.deepStrictEqual(seen, [
57+
'a:none', 'b:none', 'c:none', 'flush:none',
58+
]);
59+
}
60+
})().then(common.mustCall());
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
'use strict';
2+
3+
// A write parked on backpressure must run transform() and the writes queued
4+
// behind it in the async context of the writer, not in the context of the
5+
// read that lifts the backpressure.
6+
// Refs: https://github.com/nodejs/node/issues/66906
7+
8+
const common = require('../common');
9+
const assert = require('assert');
10+
const { AsyncLocalStorage } = require('async_hooks');
11+
const { setImmediate } = require('timers/promises');
12+
13+
const als = new AsyncLocalStorage();
14+
15+
function createPipeline(seen) {
16+
return new ReadableStream({
17+
start(controller) {
18+
for (const chunk of 'abc') controller.enqueue(chunk);
19+
controller.close();
20+
},
21+
}).pipeThrough(new TransformStream({
22+
transform(chunk, controller) {
23+
seen.push(`${chunk}:${als.getStore() ?? 'none'}`);
24+
controller.enqueue(chunk);
25+
},
26+
flush() {
27+
seen.push(`flush:${als.getStore() ?? 'none'}`);
28+
},
29+
}));
30+
}
31+
32+
async function consume(readable) {
33+
// eslint-disable-next-line no-unused-vars
34+
for await (const _ of readable);
35+
}
36+
37+
(async () => {
38+
{
39+
const seen = [];
40+
const readable = als.run('writer', () => createPipeline(seen));
41+
// Let the pipe park its first write on backpressure.
42+
await setImmediate();
43+
await consume(readable);
44+
assert.deepStrictEqual(seen, [
45+
'a:writer', 'b:writer', 'c:writer', 'flush:writer',
46+
]);
47+
}
48+
49+
{
50+
const seen = [];
51+
const readable = createPipeline(seen);
52+
await setImmediate();
53+
await als.run('reader', () => consume(readable));
54+
assert.deepStrictEqual(seen, [
55+
'a:none', 'b:none', 'c:none', 'flush:none',
56+
]);
57+
}
58+
})().then(common.mustCall());

0 commit comments

Comments
 (0)