From 8cd9b3d6ac17ad81295d8cb7d2eeccd47140d590 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 15 Mar 2020 01:34:01 +0100 Subject: [PATCH 1/3] http: don't emit 'readable' after 'close' --- lib/_http_server.js | 2 ++ test/parallel/test-async-hooks-http-parser-destroy.js | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/lib/_http_server.js b/lib/_http_server.js index 3f9c98e8380363..ec5807c5b92f8e 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -687,6 +687,8 @@ function resOnFinish(req, res, socket, state, server) { req._dump(); res.detachSocket(socket); + // TODO(ronag): streamify IncomingMessage.destroy() + req._readableState.destroyed = true; req.emit('close'); process.nextTick(emitCloseNT, res); diff --git a/test/parallel/test-async-hooks-http-parser-destroy.js b/test/parallel/test-async-hooks-http-parser-destroy.js index 22b5a0ff11b889..c077b865a84ae1 100644 --- a/test/parallel/test-async-hooks-http-parser-destroy.js +++ b/test/parallel/test-async-hooks-http-parser-destroy.js @@ -45,6 +45,13 @@ async_hooks.createHook({ }).enable(); const server = http.createServer((req, res) => { + let closed = false; + req.on('close', () => { + closed = true; + }); + req.on('readable', () => { + assert.strictEqual(closed, false); + }); res.end('Hello'); }); From c48885aa10e50c1a2020870b8cae3c9ae562a9eb Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 15 Mar 2020 13:31:57 +0100 Subject: [PATCH 2/3] fixup --- lib/_http_server.js | 8 ++++---- test/parallel/test-http-no-read-no-dump.js | 9 +++++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/_http_server.js b/lib/_http_server.js index ec5807c5b92f8e..74d01d633569df 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -665,9 +665,12 @@ function clearIncoming(req) { if (parser && parser.incoming === req) { if (req.readableEnded) { parser.incoming = null; + req.emit('close'); } else { req.on('end', clearIncoming); } + } else { + req.emit('close'); } } @@ -678,7 +681,6 @@ function resOnFinish(req, res, socket, state, server) { assert(state.incoming.length === 0 || state.incoming[0] === req); state.incoming.shift(); - clearIncoming(req); // If the user never called req.read(), and didn't pipe() or // .resume() or .on('data'), then we call req._dump() so that the @@ -687,9 +689,7 @@ function resOnFinish(req, res, socket, state, server) { req._dump(); res.detachSocket(socket); - // TODO(ronag): streamify IncomingMessage.destroy() - req._readableState.destroyed = true; - req.emit('close'); + clearIncoming(req); process.nextTick(emitCloseNT, res); if (res._last) { diff --git a/test/parallel/test-http-no-read-no-dump.js b/test/parallel/test-http-no-read-no-dump.js index 17d36c56b2eeba..c3fa7103a2f79f 100644 --- a/test/parallel/test-http-no-read-no-dump.js +++ b/test/parallel/test-http-no-read-no-dump.js @@ -1,6 +1,7 @@ 'use strict'; const common = require('../common'); const http = require('http'); +const assert = require('assert'); let onPause = null; @@ -11,6 +12,14 @@ const server = http.createServer((req, res) => { res.writeHead(200); res.flushHeaders(); + let closed = false; + req.on('close', common.mustCall(() => { + closed = true; + })); + req.on('end', common.mustCall(() => { + assert.strictEqual(closed, false); + })); + req.connection.on('pause', () => { res.end(); onPause(); From 0d9b733ca1a9280b1699fb1fc5d5ecd992d53919 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Thu, 19 Mar 2020 22:52:21 +0100 Subject: [PATCH 3/3] fixup: test --- .../parallel/test-async-hooks-http-parser-destroy.js | 12 ++++-------- test/parallel/test-http-no-read-no-dump.js | 7 +------ 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/test/parallel/test-async-hooks-http-parser-destroy.js b/test/parallel/test-async-hooks-http-parser-destroy.js index c077b865a84ae1..6cbc2043b09583 100644 --- a/test/parallel/test-async-hooks-http-parser-destroy.js +++ b/test/parallel/test-async-hooks-http-parser-destroy.js @@ -1,5 +1,5 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const async_hooks = require('async_hooks'); const http = require('http'); @@ -45,13 +45,9 @@ async_hooks.createHook({ }).enable(); const server = http.createServer((req, res) => { - let closed = false; - req.on('close', () => { - closed = true; - }); - req.on('readable', () => { - assert.strictEqual(closed, false); - }); + req.on('close', common.mustCall(() => { + req.on('readable', common.mustNotCall()); + })); res.end('Hello'); }); diff --git a/test/parallel/test-http-no-read-no-dump.js b/test/parallel/test-http-no-read-no-dump.js index c3fa7103a2f79f..c6ce19845d80ab 100644 --- a/test/parallel/test-http-no-read-no-dump.js +++ b/test/parallel/test-http-no-read-no-dump.js @@ -1,7 +1,6 @@ 'use strict'; const common = require('../common'); const http = require('http'); -const assert = require('assert'); let onPause = null; @@ -12,12 +11,8 @@ const server = http.createServer((req, res) => { res.writeHead(200); res.flushHeaders(); - let closed = false; req.on('close', common.mustCall(() => { - closed = true; - })); - req.on('end', common.mustCall(() => { - assert.strictEqual(closed, false); + req.on('end', common.mustNotCall()); })); req.connection.on('pause', () => {