Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 8a2b62e

Browse filesBrowse files
ronagcodebytere
authored andcommitted
stream: ensure pipeline always destroys streams
There was an edge case where an incorrect assumption was made in regardos whether eos/finished means that the stream is actually destroyed or not. Backport-PR-URL: #31975 PR-URL: #31940 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 313ecaa commit 8a2b62e
Copy full SHA for 8a2b62e

File tree

Expand file treeCollapse file tree

2 files changed

+19
-13
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+19
-13
lines changed
Open diff view settings
Collapse file

‎lib/internal/streams/pipeline.js‎

Copy file name to clipboardExpand all lines: lib/internal/streams/pipeline.js
+5-12Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,27 +38,20 @@ function destroyStream(stream, err) {
3838

3939
function destroyer(stream, reading, writing, callback) {
4040
callback = once(callback);
41-
42-
let closed = false;
43-
stream.on('close', () => {
44-
closed = true;
45-
});
41+
let destroyed = false;
4642

4743
if (eos === undefined) eos = require('internal/streams/end-of-stream');
4844
eos(stream, { readable: reading, writable: writing }, (err) => {
49-
if (err) return callback(err);
50-
closed = true;
51-
callback();
45+
if (destroyed) return;
46+
destroyed = true;
47+
destroyStream(stream, err);
48+
callback(err);
5249
});
5350

54-
let destroyed = false;
5551
return (err) => {
56-
if (closed) return;
5752
if (destroyed) return;
5853
destroyed = true;
59-
6054
destroyStream(stream, err);
61-
6255
callback(err || new ERR_STREAM_DESTROYED('pipe'));
6356
};
6457
}
Collapse file

‎test/parallel/test-stream-pipeline.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-stream-pipeline.js
+14-1Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -763,7 +763,10 @@ const { promisify } = require('util');
763763
s.emit('data', 'asd');
764764
s.emit('end');
765765
});
766-
s.close = common.mustCall();
766+
// 'destroyer' can be called multiple times,
767+
// once from stream wrapper and
768+
// once from iterator wrapper.
769+
s.close = common.mustCallAtLeast(1);
767770
let ret = '';
768771
pipeline(s, async function(source) {
769772
for await (const chunk of source) {
@@ -909,3 +912,13 @@ const { promisify } = require('util');
909912
assert.strictEqual(err.message, 'kaboom');
910913
}));
911914
}
915+
916+
{
917+
const src = new PassThrough({ autoDestroy: false });
918+
const dst = new PassThrough({ autoDestroy: false });
919+
pipeline(src, dst, common.mustCall(() => {
920+
assert.strictEqual(src.destroyed, true);
921+
assert.strictEqual(dst.destroyed, true);
922+
}));
923+
src.end();
924+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.