From 87818dc8bc7df3d9dda9d736b9951ab0c26773e6 Mon Sep 17 00:00:00 2001 From: Mathias Buus Date: Fri, 6 Apr 2018 17:50:58 +0200 Subject: [PATCH] http2: destroy the socket properly and add tests Fix a bug where the socket wasn't being correctly destroyed and adjust existing tests, as well as add additional tests. PR-URL: https://github.com/nodejs/node/pull/19852 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat Co-authored-by: Matteo Collina --- lib/internal/http2/core.js | 2 +- .../test-http2-many-writes-and-destroy.js | 30 +++++++++++++++++++ test/parallel/test-http2-pipe.js | 1 + .../test-http2-server-close-callback.js | 24 +++++++++++++++ ...est-http2-server-stream-session-destroy.js | 12 ++++++-- test/parallel/test-http2-session-unref.js | 14 ++++++--- .../test-http2-max-session-memory.js | 4 +++ 7 files changed, 79 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-http2-many-writes-and-destroy.js create mode 100644 test/parallel/test-http2-server-close-callback.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 1afaa00126c997..3b064749322ea1 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1150,7 +1150,7 @@ class Http2Session extends EventEmitter { // Otherwise, destroy immediately. if (!socket.destroyed) { if (!error) { - setImmediate(socket.end.bind(socket)); + setImmediate(socket.destroy.bind(socket)); } else { socket.destroy(error); } diff --git a/test/parallel/test-http2-many-writes-and-destroy.js b/test/parallel/test-http2-many-writes-and-destroy.js new file mode 100644 index 00000000000000..78db76e001cd3a --- /dev/null +++ b/test/parallel/test-http2-many-writes-and-destroy.js @@ -0,0 +1,30 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http2 = require('http2'); + +{ + const server = http2.createServer((req, res) => { + req.pipe(res); + }); + + server.listen(0, () => { + const url = `http://localhost:${server.address().port}`; + const client = http2.connect(url); + const req = client.request({ ':method': 'POST' }); + + for (let i = 0; i < 4000; i++) { + req.write(Buffer.alloc(6)); + } + + req.on('close', common.mustCall(() => { + console.log('(req onclose)'); + server.close(); + client.close(); + })); + + req.once('data', common.mustCall(() => req.destroy())); + }); +} diff --git a/test/parallel/test-http2-pipe.js b/test/parallel/test-http2-pipe.js index 2a759f9848721b..d7dd99df91edb5 100644 --- a/test/parallel/test-http2-pipe.js +++ b/test/parallel/test-http2-pipe.js @@ -33,6 +33,7 @@ server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}`); const req = client.request({ ':method': 'POST' }); + req.on('response', common.mustCall()); req.resume(); diff --git a/test/parallel/test-http2-server-close-callback.js b/test/parallel/test-http2-server-close-callback.js new file mode 100644 index 00000000000000..66887aa62bebe5 --- /dev/null +++ b/test/parallel/test-http2-server-close-callback.js @@ -0,0 +1,24 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const http2 = require('http2'); + +const server = http2.createServer(); + +server.listen(0, common.mustCall(() => { + const client = http2.connect(`http://localhost:${server.address().port}`); + client.on('error', (err) => { + if (err.code !== 'ECONNRESET') + throw err; + }); +})); + +server.on('session', common.mustCall((s) => { + setImmediate(() => { + server.close(common.mustCall()); + s.destroy(); + }); +})); diff --git a/test/parallel/test-http2-server-stream-session-destroy.js b/test/parallel/test-http2-server-stream-session-destroy.js index 5eb04a8d376635..e70630fe0b1351 100644 --- a/test/parallel/test-http2-server-stream-session-destroy.js +++ b/test/parallel/test-http2-server-stream-session-destroy.js @@ -39,10 +39,16 @@ server.on('stream', common.mustCall((stream) => { server.listen(0, common.mustCall(() => { const client = h2.connect(`http://localhost:${server.address().port}`); - client.on('error', () => {}); + client.on('error', (err) => { + if (err.code !== 'ECONNRESET') + throw err; + }); const req = client.request(); req.resume(); req.on('end', common.mustCall()); - req.on('close', common.mustCall(() => server.close())); - req.on('error', () => {}); + req.on('close', common.mustCall(() => server.close(common.mustCall()))); + req.on('error', (err) => { + if (err.code !== 'ECONNRESET') + throw err; + }); })); diff --git a/test/parallel/test-http2-session-unref.js b/test/parallel/test-http2-session-unref.js index e765352cdc615d..465f01d0921f25 100644 --- a/test/parallel/test-http2-session-unref.js +++ b/test/parallel/test-http2-session-unref.js @@ -34,8 +34,11 @@ server.listen(0, common.mustCall(() => { // unref destroyed client { const client = http2.connect(`http://localhost:${port}`); - client.destroy(); - client.unref(); + + client.on('connect', common.mustCall(() => { + client.destroy(); + client.unref(); + })); } // unref destroyed client @@ -43,8 +46,11 @@ server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${port}`, { createConnection: common.mustCall(() => clientSide) }); - client.destroy(); - client.unref(); + + client.on('connect', common.mustCall(() => { + client.destroy(); + client.unref(); + })); } })); server.emit('connection', serverSide); diff --git a/test/sequential/test-http2-max-session-memory.js b/test/sequential/test-http2-max-session-memory.js index d6d3bf935db801..644a20a3c88a50 100644 --- a/test/sequential/test-http2-max-session-memory.js +++ b/test/sequential/test-http2-max-session-memory.js @@ -13,6 +13,10 @@ const largeBuffer = Buffer.alloc(1e6); const server = http2.createServer({ maxSessionMemory: 1 }); server.on('stream', common.mustCall((stream) => { + stream.on('error', (err) => { + if (err.code !== 'ECONNRESET') + throw err; + }); stream.respond(); stream.end(largeBuffer); }));