From 02c4869beec52d2664c747d520dfe078d2b3c714 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 21 Jul 2020 07:45:11 +0200 Subject: [PATCH] stream: fix Duplex._construct race Ensures that _construct has finished before invoking _destroy. The 'constructed' property was not properly set to false for both writable and readable state. Fixes: https://github.com/nodejs/node/issues/34448 PR-URL: https://github.com/nodejs/node/pull/34456 Reviewed-By: James M Snell Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca --- lib/internal/streams/destroy.js | 14 +++++++------- test/parallel/test-stream-construct.js | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/lib/internal/streams/destroy.js b/lib/internal/streams/destroy.js index a2a91245ca46ea..3bccd46b7e65da 100644 --- a/lib/internal/streams/destroy.js +++ b/lib/internal/streams/destroy.js @@ -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; @@ -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); } diff --git a/test/parallel/test-stream-construct.js b/test/parallel/test-stream-construct.js index a3e2b4d2e3e8bd..9b38f30a01f8c2 100644 --- a/test/parallel/test-stream-construct.js +++ b/test/parallel/test-stream-construct.js @@ -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); + })); +}