From 2df7396e27b08cabf66ca01bfc9185736dadb480 Mon Sep 17 00:00:00 2001 From: Mathias Buus Date: Fri, 6 Apr 2018 17:50:58 +0200 Subject: [PATCH 1/4] http2: add failing writes with destroy test --- .../test-http2-many-writes-and-destroy.js | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 test/parallel/test-http2-many-writes-and-destroy.js 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())); + }); +} From e7b7035db3c400178ef8183b688498819c9b1cc1 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 11 Apr 2018 17:32:39 +0200 Subject: [PATCH 2/4] http2: destroy the underlining socket on destroy --- lib/internal/http2/core.js | 2 +- test/parallel/test-http2-session-unref.js | 14 ++++++++++---- test/sequential/test-http2-max-session-memory.js | 4 ++++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index ede4f5d34371e8..8b085b3a9fda99 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -1176,7 +1176,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-session-unref.js b/test/parallel/test-http2-session-unref.js index e63cd0d208e32b..f946c2d3377b87 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..6013bf95102c4c 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', common.expectsError({ + code: 'ECONNRESET', + type: Error + })); stream.respond(); stream.end(largeBuffer); })); From 430c03a0921a2cf68cf70c0da3e4aa74961cea1b Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 10 May 2018 13:49:17 +0200 Subject: [PATCH 3/4] http2: added close test, fixed flaky http2-pipe --- test/parallel/test-http2-pipe.js | 9 +++++++ .../test-http2-server-close-callback.js | 26 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 test/parallel/test-http2-server-close-callback.js diff --git a/test/parallel/test-http2-pipe.js b/test/parallel/test-http2-pipe.js index 2a759f9848721b..a8974a3623a7b9 100644 --- a/test/parallel/test-http2-pipe.js +++ b/test/parallel/test-http2-pipe.js @@ -21,6 +21,10 @@ const server = http2.createServer(); server.on('stream', common.mustCall((stream) => { const dest = stream.pipe(fs.createWriteStream(fn)); + // we might get an ECONNRESET here + // or not + stream.on('error', () => {}); + dest.on('finish', () => { assert.strictEqual(fs.readFileSync(loc).length, fs.readFileSync(fn).length); @@ -33,6 +37,11 @@ server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}`); const req = client.request({ ':method': 'POST' }); + + // we might get an ECONNRESET here + // or not + req.on('error', () => {}); + 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..226319fae6491b --- /dev/null +++ b/test/parallel/test-http2-server-close-callback.js @@ -0,0 +1,26 @@ +'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}`); + + // depending on network events, this might or might not be emitted + client.on('error', () => {}); + + client.on('connect', common.mustCall(() => { + server.close(common.mustCall()); + })); +})); + +server.on('session', common.mustCall((s) => { + setImmediate(() => { + s.destroy(); + }); +})); From 53eb58451a09db398505ffbb239ad9b098542ae5 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Thu, 10 May 2018 09:21:27 +0200 Subject: [PATCH 4/4] test: fix flaky http2 tests --- test/parallel/test-http2-pipe.js | 8 -------- test/parallel/test-http2-server-close-callback.js | 12 +++++------- .../test-http2-server-stream-session-destroy.js | 12 +++++++++--- test/sequential/test-http2-max-session-memory.js | 8 ++++---- 4 files changed, 18 insertions(+), 22 deletions(-) diff --git a/test/parallel/test-http2-pipe.js b/test/parallel/test-http2-pipe.js index a8974a3623a7b9..d7dd99df91edb5 100644 --- a/test/parallel/test-http2-pipe.js +++ b/test/parallel/test-http2-pipe.js @@ -21,10 +21,6 @@ const server = http2.createServer(); server.on('stream', common.mustCall((stream) => { const dest = stream.pipe(fs.createWriteStream(fn)); - // we might get an ECONNRESET here - // or not - stream.on('error', () => {}); - dest.on('finish', () => { assert.strictEqual(fs.readFileSync(loc).length, fs.readFileSync(fn).length); @@ -38,10 +34,6 @@ server.listen(0, common.mustCall(() => { const req = client.request({ ':method': 'POST' }); - // we might get an ECONNRESET here - // or not - req.on('error', () => {}); - 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 index 226319fae6491b..66887aa62bebe5 100644 --- a/test/parallel/test-http2-server-close-callback.js +++ b/test/parallel/test-http2-server-close-callback.js @@ -10,17 +10,15 @@ const server = http2.createServer(); server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}`); - - // depending on network events, this might or might not be emitted - client.on('error', () => {}); - - client.on('connect', common.mustCall(() => { - server.close(common.mustCall()); - })); + 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 989c72ec959679..ef4f42fe39bc3a 100644 --- a/test/parallel/test-http2-server-stream-session-destroy.js +++ b/test/parallel/test-http2-server-stream-session-destroy.js @@ -44,10 +44,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/sequential/test-http2-max-session-memory.js b/test/sequential/test-http2-max-session-memory.js index 6013bf95102c4c..644a20a3c88a50 100644 --- a/test/sequential/test-http2-max-session-memory.js +++ b/test/sequential/test-http2-max-session-memory.js @@ -13,10 +13,10 @@ const largeBuffer = Buffer.alloc(1e6); const server = http2.createServer({ maxSessionMemory: 1 }); server.on('stream', common.mustCall((stream) => { - stream.on('error', common.expectsError({ - code: 'ECONNRESET', - type: Error - })); + stream.on('error', (err) => { + if (err.code !== 'ECONNRESET') + throw err; + }); stream.respond(); stream.end(largeBuffer); }));