Skip to content

Commit 0b933ff

Browse files
committed
stream/iter: fix pipeToSync byte accounting
Stop pipeToSync() from counting chunks when writeSync() or writevSync() returns false without accepting the data. Preserve writers that use false as an accepted backpressure signal. Fixes: #63562 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5
1 parent 15d0c61 commit 0b933ff

2 files changed

Lines changed: 75 additions & 13 deletions

File tree

lib/internal/streams/iter/pull.js

Lines changed: 36 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,18 @@ async function* createAsyncPipeline(source, transforms, signal) {
723723
}
724724
}
725725

726+
/**
727+
* Check if a false sync write result means accepted backpressure.
728+
* @param {object} writer - The writer whose sync method returned.
729+
* @param {*} result - The return value from writeSync() or writevSync().
730+
* @returns {boolean}
731+
*/
732+
function isAcceptedSyncWriteBackpressure(writer, result) {
733+
return result === false &&
734+
writer[kSyncWriteAcceptedOnFalse] === true &&
735+
writer.desiredSize === 0;
736+
}
737+
726738
// =============================================================================
727739
// Public API: pull() and pullSync()
728740
// =============================================================================
@@ -810,16 +822,29 @@ function pipeToSync(source, ...args) {
810822
const hasEndSync = typeof writer.endSync === 'function';
811823

812824
try {
825+
let canContinue = true;
813826
for (const batch of pipeline) {
827+
if (!canContinue) {
828+
break;
829+
}
814830
if (hasWritevSync && batch.length > 1) {
815-
writer.writevSync(batch);
831+
const result = writer.writevSync(batch);
832+
if (result === false &&
833+
!isAcceptedSyncWriteBackpressure(writer, result)) {
834+
break;
835+
}
816836
for (let i = 0; i < batch.length; i++) {
817837
totalBytes += TypedArrayPrototypeGetByteLength(batch[i]);
818838
}
819839
} else {
820840
for (let i = 0; i < batch.length; i++) {
821841
const chunk = batch[i];
822-
writer.writeSync(chunk);
842+
const result = writer.writeSync(chunk);
843+
if (result === false &&
844+
!isAcceptedSyncWriteBackpressure(writer, result)) {
845+
canContinue = false;
846+
break;
847+
}
823848
totalBytes += TypedArrayPrototypeGetByteLength(chunk);
824849
}
825850
}
@@ -871,11 +896,6 @@ async function pipeTo(source, ...args) {
871896
const hasWritev = typeof writer.writev === 'function';
872897
const hasWritevSync = typeof writer.writevSync === 'function';
873898
const hasEndSync = typeof writer.endSync === 'function';
874-
const syncFalseCanBeAccepted = writer[kSyncWriteAcceptedOnFalse] === true;
875-
876-
function syncFalseWasAccepted() {
877-
return syncFalseCanBeAccepted && writer.desiredSize === 0;
878-
}
879899

880900
function waitForSyncBackpressure() {
881901
const ondrain = writer[drainableProtocol];
@@ -892,9 +912,10 @@ async function pipeTo(source, ...args) {
892912
async function writeBatchAsyncFallback(batch, startIndex) {
893913
for (let i = startIndex; i < batch.length; i++) {
894914
const chunk = batch[i];
895-
if (hasWriteSync && writer.writeSync(chunk)) {
915+
const result = hasWriteSync && writer.writeSync(chunk);
916+
if (result) {
896917
// Sync retry succeeded
897-
} else if (syncFalseWasAccepted()) {
918+
} else if (isAcceptedSyncWriteBackpressure(writer, result)) {
898919
totalBytes += TypedArrayPrototypeGetByteLength(chunk);
899920
await waitForSyncBackpressure();
900921
continue;
@@ -914,8 +935,9 @@ async function pipeTo(source, ...args) {
914935
// is required. Callers must check: const p = writeBatch(b); if (p) await p;
915936
function writeBatch(batch) {
916937
if (hasWritev && batch.length > 1) {
917-
if (!hasWritevSync || !writer.writevSync(batch)) {
918-
if (hasWritevSync && syncFalseWasAccepted()) {
938+
const result = hasWritevSync && writer.writevSync(batch);
939+
if (!result) {
940+
if (isAcceptedSyncWriteBackpressure(writer, result)) {
919941
for (let i = 0; i < batch.length; i++) {
920942
totalBytes += TypedArrayPrototypeGetByteLength(batch[i]);
921943
}
@@ -935,8 +957,9 @@ async function pipeTo(source, ...args) {
935957
}
936958
for (let i = 0; i < batch.length; i++) {
937959
const chunk = batch[i];
938-
if (!hasWriteSync || !writer.writeSync(chunk)) {
939-
if (hasWriteSync && syncFalseWasAccepted()) {
960+
const result = hasWriteSync && writer.writeSync(chunk);
961+
if (!result) {
962+
if (isAcceptedSyncWriteBackpressure(writer, result)) {
940963
totalBytes += TypedArrayPrototypeGetByteLength(chunk);
941964
return writeBatchAfterAcceptedBackpressure(batch, i + 1);
942965
}

test/parallel/test-stream-iter-pipeto-writev.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,43 @@ async function testPushWriterBlockSyncFalseAccepted() {
134134
})(), 'abcd', 4);
135135
}
136136

137+
async function testPipeToSyncPushWriterStrictFalseRejected() {
138+
const decoder = new TextDecoder();
139+
const { writer, readable } = push({ highWaterMark: 1 });
140+
141+
const total = pipeToSync(['a', 'b'], writer, { preventClose: true });
142+
assert.strictEqual(total, 1);
143+
144+
const iter = readable[Symbol.asyncIterator]();
145+
const first = await iter.next();
146+
assert.strictEqual(first.done, false);
147+
assert.strictEqual(decoder.decode(first.value[0]), 'a');
148+
149+
const second = await Promise.race([
150+
iter.next().then((result) => {
151+
return result.done ? '<done>' : decoder.decode(result.value[0]);
152+
}),
153+
setImmediatePromise().then(() => '<no second chunk>'),
154+
]);
155+
assert.strictEqual(second, '<no second chunk>');
156+
157+
await iter.return?.();
158+
}
159+
160+
async function testPipeToSyncWritevFalseNotCounted() {
161+
const writer = {
162+
writevSync() { return false; },
163+
writeSync: common.mustNotCall(),
164+
endSync() { return 0; },
165+
};
166+
function* source() {
167+
yield [new Uint8Array([1]), new Uint8Array([2])];
168+
}
169+
170+
const total = pipeToSync(source(), writer);
171+
assert.strictEqual(total, 0);
172+
}
173+
137174
// pipeToSync with writevSync
138175
async function testPipeToSyncWritev() {
139176
const batches = [];
@@ -194,6 +231,8 @@ Promise.all([
194231
testWriteSyncFailsMidBatch(),
195232
testWriteSyncAlwaysFails(),
196233
testPushWriterBlockSyncFalseAccepted(),
234+
testPipeToSyncPushWriterStrictFalseRejected(),
235+
testPipeToSyncWritevFalseNotCounted(),
197236
testPipeToSyncWritev(),
198237
testPipeToSyncPlainChunksWritev(),
199238
testPipeToSyncWriteFallback(),

0 commit comments

Comments
 (0)