From 6ae4e4a86814f6dac68e0f603ba26fc41cc9939a Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 13 Apr 2021 00:54:24 +0200 Subject: [PATCH 1/3] stream: fix multiple Writable.destroy() calls. Calling Writable.destroy() multiple times in the same tick could cause an assertion error. Fixes: https://github.com/nodejs/node/issues/38189 --- lib/internal/streams/writable.js | 4 ++-- test/parallel/test-stream-writable-destroy.js | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/internal/streams/writable.js b/lib/internal/streams/writable.js index 2fe811236a27e8..693ef2ee5d25ff 100644 --- a/lib/internal/streams/writable.js +++ b/lib/internal/streams/writable.js @@ -855,10 +855,10 @@ Writable.prototype.destroy = function(err, cb) { // Invoke pending callbacks. if ( + !state.destroyed && ( state.bufferedIndex < state.buffered.length || state[kOnFinished].length - ) { - assert(!state.destroyed); + )) { process.nextTick(errorBuffer, state); } diff --git a/test/parallel/test-stream-writable-destroy.js b/test/parallel/test-stream-writable-destroy.js index 7cd800b0938138..2e6e1f975a2be1 100644 --- a/test/parallel/test-stream-writable-destroy.js +++ b/test/parallel/test-stream-writable-destroy.js @@ -460,3 +460,14 @@ const assert = require('assert'); assert.strictEqual(write.destroyed, true); })); } + +{ + // Destroy twice + const write = new Writable({ + write(chunk, enc, cb) { cb(); } + }); + + write.end(common.mustCall()); + write.destroy(); + write.destroy(); +} From 9bc770384811b4e7179bd95525acba93240b09c3 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 13 Apr 2021 12:08:39 +0200 Subject: [PATCH 2/3] fixup: linting --- lib/internal/streams/writable.js | 9 +++---- test/parallel/test-http-many-requests.js | 31 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-http-many-requests.js diff --git a/lib/internal/streams/writable.js b/lib/internal/streams/writable.js index 693ef2ee5d25ff..d00e175ceb1c1b 100644 --- a/lib/internal/streams/writable.js +++ b/lib/internal/streams/writable.js @@ -57,7 +57,6 @@ const { getHighWaterMark, getDefaultHighWaterMark } = require('internal/streams/state'); -const assert = require('internal/assert'); const { ERR_INVALID_ARG_TYPE, ERR_METHOD_NOT_IMPLEMENTED, @@ -854,11 +853,9 @@ Writable.prototype.destroy = function(err, cb) { const state = this._writableState; // Invoke pending callbacks. - if ( - !state.destroyed && ( - state.bufferedIndex < state.buffered.length || - state[kOnFinished].length - )) { + if (!state.destroyed && + (state.bufferedIndex < state.buffered.length || + state[kOnFinished].length)) { process.nextTick(errorBuffer, state); } diff --git a/test/parallel/test-http-many-requests.js b/test/parallel/test-http-many-requests.js new file mode 100644 index 00000000000000..b1288a6df0b59d --- /dev/null +++ b/test/parallel/test-http-many-requests.js @@ -0,0 +1,31 @@ +'use strict'; + +const common = require('../common'); +const http = require('http'); +const Countdown = require('../common/countdown'); + +const NUM_REQ = 128 + +const agent = new http.Agent({ keepAlive: true }); +const countdown = new Countdown(NUM_REQ, () => server.close()); + +const server = http.createServer(common.mustCall(function(req, res) { + res.setHeader('content-type', 'application/json; charset=utf-8') + res.end(JSON.stringify({ hello: 'world' })) +}, NUM_REQ)).listen(0, function() { + for (let i = 0; i < NUM_REQ; ++i) { + const req = http.request({ + port: server.address().port, + agent, + method: 'GET' + }, function(res) { + res.resume(); + res.on('end', () => { + countdown.dec(); + }) + }) + .end(); + } +}); + +process.on('warning', common.mustNotCall()); From 8fc0ef2c4d744b3f2d727b579da65e1d766364c1 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Wed, 14 Apr 2021 09:24:00 +0200 Subject: [PATCH 3/3] fixup --- test/parallel/test-http-many-requests.js | 31 ------------------------ 1 file changed, 31 deletions(-) delete mode 100644 test/parallel/test-http-many-requests.js diff --git a/test/parallel/test-http-many-requests.js b/test/parallel/test-http-many-requests.js deleted file mode 100644 index b1288a6df0b59d..00000000000000 --- a/test/parallel/test-http-many-requests.js +++ /dev/null @@ -1,31 +0,0 @@ -'use strict'; - -const common = require('../common'); -const http = require('http'); -const Countdown = require('../common/countdown'); - -const NUM_REQ = 128 - -const agent = new http.Agent({ keepAlive: true }); -const countdown = new Countdown(NUM_REQ, () => server.close()); - -const server = http.createServer(common.mustCall(function(req, res) { - res.setHeader('content-type', 'application/json; charset=utf-8') - res.end(JSON.stringify({ hello: 'world' })) -}, NUM_REQ)).listen(0, function() { - for (let i = 0; i < NUM_REQ; ++i) { - const req = http.request({ - port: server.address().port, - agent, - method: 'GET' - }, function(res) { - res.resume(); - res.on('end', () => { - countdown.dec(); - }) - }) - .end(); - } -}); - -process.on('warning', common.mustNotCall());