Skip to content

Commit

Permalink
stream: fix Duplex._construct race
Browse files Browse the repository at this point in the history
Ensures that _construct has finished before invoking
_destroy.

The 'constructed' property was not properly set to false
for both writable and readable state.

Fixes: #34448

PR-URL: #34456
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
ronag authored and jasnell committed Jul 21, 2020
1 parent 95770df commit 02c4869
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 7 deletions.
14 changes: 7 additions & 7 deletions lib/internal/streams/destroy.js
Expand Up @@ -203,13 +203,6 @@ function construct(stream, cb) {
return;
}

stream.once(kConstruct, cb);

if (stream.listenerCount(kConstruct) > 1) {
// Duplex
return;
}

const r = stream._readableState;
const w = stream._writableState;

Expand All @@ -220,6 +213,13 @@ function construct(stream, cb) {
w.constructed = false;
}

stream.once(kConstruct, cb);

if (stream.listenerCount(kConstruct) > 1) {
// Duplex
return;
}

process.nextTick(constructNT, stream);
}

Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-stream-construct.js
Expand Up @@ -242,3 +242,29 @@ testDestroy((opts) => new Writable({
construct: common.mustCall()
});
}

{
// https://github.com/nodejs/node/issues/34448

let constructed = false;
const d = new Duplex({
readable: false,
construct: common.mustCall((callback) => {
setImmediate(common.mustCall(() => {
constructed = true;
callback();
}));
}),
write(chunk, encoding, callback) {
callback();
},
read() {
this.push(null);
}
});
d.resume();
d.end('foo');
d.on('close', common.mustCall(() => {
assert.strictEqual(constructed, true);
}));
}

0 comments on commit 02c4869

Please sign in to comment.