Skip to content

Commit 8a2b62e

Browse files
ronagcodebytere
authored andcommittedMar 1, 2020
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

File tree

2 files changed

+19
-13
lines changed

2 files changed

+19
-13
lines changed
 

‎lib/internal/streams/pipeline.js

+5-12
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
}

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

+14-1
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)
Please sign in to comment.