From 7809adfb1f8f6b85d057e1a88af3e553a0f99186 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Fri, 30 Aug 2019 12:13:58 +0200 Subject: [PATCH] stream: don't deadlock on aborted stream Not all streams (e.g. http.ClientRequest) will always emit 'close' after 'aborted'. PR-URL: https://github.com/nodejs/node/pull/29376 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- lib/internal/streams/end-of-stream.js | 6 +++++ test/parallel/test-http-client-finished.js | 27 ++++++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 test/parallel/test-http-client-finished.js diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js index 850e5d3796c448..3f1c0f316cd3c6 100644 --- a/lib/internal/streams/end-of-stream.js +++ b/lib/internal/streams/end-of-stream.js @@ -82,12 +82,18 @@ function eos(stream, opts, callback) { stream.on('close', onlegacyfinish); } + // Not all streams will emit 'close' after 'aborted'. + if (typeof stream.aborted === 'boolean') { + stream.on('aborted', onclose); + } + stream.on('end', onend); stream.on('finish', onfinish); if (opts.error !== false) stream.on('error', onerror); stream.on('close', onclose); return function() { + stream.removeListener('aborted', onclose); stream.removeListener('complete', onfinish); stream.removeListener('abort', onclose); stream.removeListener('request', onrequest); diff --git a/test/parallel/test-http-client-finished.js b/test/parallel/test-http-client-finished.js new file mode 100644 index 00000000000000..2d7e5b95b3ca33 --- /dev/null +++ b/test/parallel/test-http-client-finished.js @@ -0,0 +1,27 @@ +'use strict'; +const common = require('../common'); +const http = require('http'); +const { finished } = require('stream'); + +{ + // Test abort before finished. + + const server = http.createServer(function(req, res) { + res.write('asd'); + }); + + server.listen(0, common.mustCall(function() { + http.request({ + port: this.address().port + }) + .on('response', (res) => { + res.on('readable', () => { + res.destroy(); + }); + finished(res, common.mustCall(() => { + server.close(); + })); + }) + .end(); + })); +}