Skip to content

Commit 6abf57e

Browse files
committed
streams: let destroyed-flagged readables still flush to piped destinations [build images]
Deliberate divergence from Node 26 (nodejs/node#62557 made pause()/resume() early-return on destroyed streams). Legacy Readable subclasses like fd-slicer assign `this.destroyed = true` — which hits the prototype setter on modern streams — right before push(null). With the upstream guard, a piped destination's 'drain' can no longer resume the source, so the last buffered chunk is silently dropped and the pipeline never finishes. In practice that breaks yauzl → extract-zip → puppeteer browser installs and other zip/tar tooling, and the same hang reproduces on Node 26.3.0 itself. Keep the Node 24 behavior of letting such streams flush their buffer, and replace the no-op assertion test with a regression test for the fd-slicer pattern (verified: extract-zip now extracts a full chrome-headless-shell archive instead of stopping after the first entries).
1 parent 09f9406 commit 6abf57e

2 files changed

Lines changed: 49 additions & 19 deletions

File tree

src/js/internal/streams/readable.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1132,9 +1132,13 @@ function nReadingNextTick(self) {
11321132
// If the user uses them, then switch into old mode.
11331133
Readable.prototype.resume = function () {
11341134
const state = this._readableState;
1135-
if ((state[kState] & kDestroyed) !== 0) {
1136-
return this;
1137-
}
1135+
// Deliberate divergence from Node 26: upstream early-returns here (and in
1136+
// pause()) when the stream is destroyed. Legacy Readable subclasses like
1137+
// fd-slicer assign `this.destroyed = true` (the prototype setter) right
1138+
// before push(null), so with the guard a piped destination's drain can no
1139+
// longer resume the source and the final buffered chunk is never delivered —
1140+
// silently truncating yauzl/extract-zip/puppeteer downloads. Keep the
1141+
// Node 24 behavior of letting destroyed streams flush their buffer.
11381142
if ((state[kState] & kFlowing) === 0) {
11391143
$debug("resume");
11401144
// We flow only if there is no one listening
@@ -1174,9 +1178,7 @@ function resume_(stream, state) {
11741178

11751179
Readable.prototype.pause = function () {
11761180
const state = this._readableState;
1177-
if ((state[kState] & kDestroyed) !== 0) {
1178-
return this;
1179-
}
1181+
// No destroyed early-return: see the comment in resume() above.
11801182
$debug("call pause");
11811183
if ((state[kState] & (kHasFlowing | kFlowing)) !== kHasFlowing) {
11821184
$debug("pause");

test/js/node/stream/node-stream.test.js

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -821,19 +821,47 @@ describe("node v26 stream semantics", () => {
821821
expect(r.read()).toBeNull();
822822
});
823823

824-
// Upstream: nodejs/node#62557 (test-stream-readable-pause-and-resume.js).
825-
it("pause() and resume() are no-ops on destroyed streams", async () => {
826-
const r = new Readable({ read() {} });
827-
r.destroy();
828-
const emitted = [];
829-
r.on("pause", () => emitted.push("pause"));
830-
r.on("resume", () => emitted.push("resume"));
831-
expect(r.resume()).toBe(r);
832-
expect(r.readableFlowing).toBeNull();
833-
expect(r.pause()).toBe(r);
834-
expect(r.readableFlowing).toBeNull();
835-
await new Promise(resolve => setImmediate(resolve));
836-
expect(emitted).toEqual([]);
824+
// Deliberate divergence from Node 26 (nodejs/node#62557 made pause/resume
825+
// no-ops on destroyed streams): legacy Readable subclasses like fd-slicer
826+
// (yauzl → extract-zip → puppeteer/electron tooling) assign
827+
// `this.destroyed = true` via the prototype setter right before push(null).
828+
// With the upstream guard, a piped destination's drain can no longer resume
829+
// the source, so the final buffered chunk is silently dropped and the
830+
// pipeline never finishes. We keep the Node 24 behavior: a destroyed-flagged
831+
// stream still flushes its buffered data to a piped destination.
832+
it("drain still resumes a source that flagged itself destroyed before EOF (fd-slicer pattern)", async () => {
833+
const { Transform } = require("node:stream");
834+
const chunks = [Buffer.alloc(65536, 1), Buffer.alloc(65536, 2), Buffer.alloc(40000, 3)];
835+
const src = new Readable({
836+
read() {
837+
const chunk = chunks.shift();
838+
if (chunk) {
839+
this.push(chunk);
840+
} else {
841+
// fd-slicer's ReadStream._read: sets the destroyed flag (which hits
842+
// the prototype setter on modern streams) and then pushes EOF.
843+
this.destroyed = true;
844+
this.push(null);
845+
}
846+
},
847+
});
848+
// Small writableHighWaterMark forces write() to return false so the pipe
849+
// pauses and must be revived by 'drain' → src.resume().
850+
const slow = new Transform({
851+
writableHighWaterMark: 1024,
852+
transform(chunk, encoding, callback) {
853+
setImmediate(() => callback(null, chunk));
854+
},
855+
});
856+
let received = 0;
857+
slow.on("data", c => (received += c.length));
858+
const ended = new Promise((resolve, reject) => {
859+
slow.on("end", resolve);
860+
slow.on("error", reject);
861+
});
862+
src.pipe(slow);
863+
await ended;
864+
expect(received).toBe(65536 * 2 + 40000);
837865
});
838866

839867
// Upstream: nodejs/node#60907 (test-stream-compose-operator.js).

0 commit comments

Comments
 (0)