Skip to content

Commit

Permalink
http2: destroy the socket properly and add tests
Browse files Browse the repository at this point in the history
Fix a bug where the socket wasn't being correctly destroyed and
adjust existing tests, as well as add additional tests.

PR-URL: #19852
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

Co-authored-by: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
2 people authored and addaleax committed May 14, 2018
1 parent 9422909 commit 28ecf93
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 8 deletions.
2 changes: 1 addition & 1 deletion lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,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);
}
Expand Down
30 changes: 30 additions & 0 deletions test/parallel/test-http2-many-writes-and-destroy.js
Original file line number Diff line number Diff line change
@@ -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()));
});
}
1 change: 1 addition & 0 deletions test/parallel/test-http2-pipe.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
24 changes: 24 additions & 0 deletions test/parallel/test-http2-server-close-callback.js
Original file line number Diff line number Diff line change
@@ -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();
});
}));
12 changes: 9 additions & 3 deletions test/parallel/test-http2-server-stream-session-destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}));
14 changes: 10 additions & 4 deletions test/parallel/test-http2-session-unref.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,23 @@ 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
{
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);
Expand Down
4 changes: 4 additions & 0 deletions test/sequential/test-http2-max-session-memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}));
Expand Down

0 comments on commit 28ecf93

Please sign in to comment.