diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index c88d9f5231977d..9c78d8ece9f58e 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -672,8 +672,6 @@ function submitShutdown(options) { function finishSessionDestroy(self, socket) { const state = self[kState]; - if (state.destroyed) - return; if (!socket.destroyed) socket.destroy(); @@ -956,6 +954,7 @@ class Http2Session extends EventEmitter { return; debug(`[${sessionName(this[kType])}] destroying nghttp2session`); state.destroying = true; + state.destroyed = false; // Unenroll the timer this.setTimeout(0, sessionOnTimeout); @@ -970,9 +969,6 @@ class Http2Session extends EventEmitter { delete this[kSocket]; delete this[kServer]; - state.destroyed = false; - state.destroying = true; - if (this[kHandle] !== undefined) this[kHandle].destroying(); diff --git a/test/parallel/test-http2-client-destroy-before-connect.js b/test/parallel/test-http2-client-destroy-before-connect.js deleted file mode 100644 index f39e808cc641ee..00000000000000 --- a/test/parallel/test-http2-client-destroy-before-connect.js +++ /dev/null @@ -1,29 +0,0 @@ -'use strict'; - -const common = require('../common'); -if (!common.hasCrypto) - common.skip('missing crypto'); -const h2 = require('http2'); - -const server = h2.createServer(); - -// we use the lower-level API here -server.on('stream', common.mustNotCall()); - -server.listen(0); - -server.on('listening', common.mustCall(() => { - - const client = h2.connect(`http://localhost:${server.address().port}`); - - const req = client.request({ ':path': '/' }); - client.destroy(); - - req.on('response', common.mustNotCall()); - req.resume(); - req.on('end', common.mustCall(() => { - server.close(); - })); - req.end(); - -})); diff --git a/test/parallel/test-http2-client-destroy-before-request.js b/test/parallel/test-http2-client-destroy-before-request.js deleted file mode 100644 index fd66dcd3866abb..00000000000000 --- a/test/parallel/test-http2-client-destroy-before-request.js +++ /dev/null @@ -1,29 +0,0 @@ -'use strict'; - -const common = require('../common'); -if (!common.hasCrypto) - common.skip('missing crypto'); -const assert = require('assert'); -const h2 = require('http2'); - -const server = h2.createServer(); - -// we use the lower-level API here -server.on('stream', common.mustNotCall()); - -server.listen(0); - -server.on('listening', common.mustCall(() => { - - const client = h2.connect(`http://localhost:${server.address().port}`); - client.destroy(); - - assert.throws(() => client.request({ ':path': '/' }), - common.expectsError({ - code: 'ERR_HTTP2_INVALID_SESSION', - message: /^The session has been destroyed$/ - })); - - server.close(); - -})); diff --git a/test/parallel/test-http2-client-destroy-goaway.js b/test/parallel/test-http2-client-destroy-goaway.js deleted file mode 100644 index 2be33049a4bc3a..00000000000000 --- a/test/parallel/test-http2-client-destroy-goaway.js +++ /dev/null @@ -1,24 +0,0 @@ -'use strict'; - -const common = require('../common'); -if (!common.hasCrypto) - common.skip('missing crypto'); -const http2 = require('http2'); - -const server = http2.createServer(); -server.on('stream', common.mustCall((stream) => { - stream.on('error', common.mustCall()); - stream.session.shutdown(); -})); - -server.listen(0, common.mustCall(() => { - const client = http2.connect(`http://localhost:${server.address().port}`); - - client.on('goaway', common.mustCall(() => { - // We ought to be able to destroy the client in here without an error - server.close(); - client.destroy(); - })); - - client.request(); -})); diff --git a/test/parallel/test-http2-client-destroy.js b/test/parallel/test-http2-client-destroy.js index 698f752b10e9a4..94f66930aac56e 100644 --- a/test/parallel/test-http2-client-destroy.js +++ b/test/parallel/test-http2-client-destroy.js @@ -6,50 +6,142 @@ if (!common.hasCrypto) const assert = require('assert'); const h2 = require('http2'); -const server = h2.createServer(); -server.listen(0); +{ + const server = h2.createServer(); + server.listen( + 0, + common.mustCall(() => { + const destroyCallbacks = [ + (client) => client.destroy(), + (client) => client.socket.destroy() + ]; -server.on('listening', common.mustCall(function() { - const port = this.address().port; + let remaining = destroyCallbacks.length; - const destroyCallbacks = [ - (client) => client.destroy(), - (client) => client.socket.destroy() - ]; + destroyCallbacks.forEach((destroyCallback) => { + const client = h2.connect(`http://localhost:${server.address().port}`); + client.on( + 'connect', + common.mustCall(() => { + const socket = client.socket; - let remaining = destroyCallbacks.length; + assert(client.socket, 'client session has associated socket'); + assert( + !client.destroyed, + 'client has not been destroyed before destroy is called' + ); + assert( + !socket.destroyed, + 'socket has not been destroyed before destroy is called' + ); - destroyCallbacks.forEach((destroyCallback) => { - const client = h2.connect(`http://localhost:${port}`); - client.on('connect', common.mustCall(() => { - const socket = client.socket; + // Ensure that 'close' event is emitted + client.on('close', common.mustCall()); - assert(client.socket, 'client session has associated socket'); - assert(!client.destroyed, - 'client has not been destroyed before destroy is called'); - assert(!socket.destroyed, - 'socket has not been destroyed before destroy is called'); + destroyCallback(client); - // Ensure that 'close' event is emitted - client.on('close', common.mustCall()); + assert( + !client.socket, + 'client.socket undefined after destroy is called' + ); - destroyCallback(client); + // Must must be closed + client.on( + 'close', + common.mustCall(() => { + assert(client.destroyed); + }) + ); - assert(!client.socket, 'client.socket undefined after destroy is called'); + // socket will close on process.nextTick + socket.on( + 'close', + common.mustCall(() => { + assert(socket.destroyed); + }) + ); - // Must must be closed - client.on('close', common.mustCall(() => { - assert(client.destroyed); - })); + if (--remaining === 0) { + server.close(); + } + }) + ); + }); + }) + ); +} - // socket will close on process.nextTick - socket.on('close', common.mustCall(() => { - assert(socket.destroyed); - })); +// test destroy before connect +{ + const server = h2.createServer(); + server.listen( + 0, + common.mustCall(() => { + const client = h2.connect(`http://localhost:${server.address().port}`); - if (--remaining === 0) { - server.close(); - } - })); - }); -})); + const req = client.request({ ':path': '/' }); + client.destroy(); + + req.on('response', common.mustNotCall()); + req.resume(); + req.on( + 'end', + common.mustCall(() => { + server.close(); + }) + ); + req.end(); + }) + ); +} + +// test destroy before request +{ + const server = h2.createServer(); + server.listen( + 0, + common.mustCall(() => { + const client = h2.connect(`http://localhost:${server.address().port}`); + client.destroy(); + + assert.throws( + () => client.request({ ':path': '/' }), + common.expectsError({ + code: 'ERR_HTTP2_INVALID_SESSION', + message: 'The session has been destroyed' + }) + ); + + server.close(); + }) + ); +} + +// test destroy before goaway +{ + const server = h2.createServer(); + server.on( + 'stream', + common.mustCall((stream) => { + stream.on('error', common.mustCall()); + stream.session.shutdown(); + }) + ); + server.listen( + 0, + common.mustCall(() => { + const client = h2.connect(`http://localhost:${server.address().port}`); + + client.on( + 'goaway', + common.mustCall(() => { + // We ought to be able to destroy the client in here without an error + server.close(); + client.destroy(); + }) + ); + + client.request(); + }) + ); +}