Skip to content

Commit 87818dc

Browse files
mafintoshmcollina
authored andcommitted
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: #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>
1 parent 71ee19e commit 87818dc

7 files changed

Lines changed: 79 additions & 8 deletions

‎lib/internal/http2/core.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1150,7 +1150,7 @@ class Http2Session extends EventEmitter {
11501150
// Otherwise, destroy immediately.
11511151
if(!socket.destroyed){
11521152
if(!error){
1153-
setImmediate(socket.end.bind(socket));
1153+
setImmediate(socket.destroy.bind(socket));
11541154
}else{
11551155
socket.destroy(error);
11561156
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
if(!common.hasCrypto)
5+
common.skip('missing crypto');
6+
consthttp2=require('http2');
7+
8+
{
9+
constserver=http2.createServer((req,res)=>{
10+
req.pipe(res);
11+
});
12+
13+
server.listen(0,()=>{
14+
consturl=`http://localhost:${server.address().port}`;
15+
constclient=http2.connect(url);
16+
constreq=client.request({':method': 'POST'});
17+
18+
for(leti=0;i<4000;i++){
19+
req.write(Buffer.alloc(6));
20+
}
21+
22+
req.on('close',common.mustCall(()=>{
23+
console.log('(req onclose)');
24+
server.close();
25+
client.close();
26+
}));
27+
28+
req.once('data',common.mustCall(()=>req.destroy()));
29+
});
30+
}

‎test/parallel/test-http2-pipe.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ server.listen(0, common.mustCall(() => {
3333
constclient=http2.connect(`http://localhost:${server.address().port}`);
3434

3535
constreq=client.request({':method': 'POST'});
36+
3637
req.on('response',common.mustCall());
3738
req.resume();
3839

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
if(!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
consthttp2=require('http2');
8+
9+
constserver=http2.createServer();
10+
11+
server.listen(0,common.mustCall(()=>{
12+
constclient=http2.connect(`http://localhost:${server.address().port}`);
13+
client.on('error',(err)=>{
14+
if(err.code!=='ECONNRESET')
15+
throwerr;
16+
});
17+
}));
18+
19+
server.on('session',common.mustCall((s)=>{
20+
setImmediate(()=>{
21+
server.close(common.mustCall());
22+
s.destroy();
23+
});
24+
}));

‎test/parallel/test-http2-server-stream-session-destroy.js‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,16 @@ server.on('stream', common.mustCall((stream) => {
3939

4040
server.listen(0,common.mustCall(()=>{
4141
constclient=h2.connect(`http://localhost:${server.address().port}`);
42-
client.on('error',()=>{});
42+
client.on('error',(err)=>{
43+
if(err.code!=='ECONNRESET')
44+
throwerr;
45+
});
4346
constreq=client.request();
4447
req.resume();
4548
req.on('end',common.mustCall());
46-
req.on('close',common.mustCall(()=>server.close()));
47-
req.on('error',()=>{});
49+
req.on('close',common.mustCall(()=>server.close(common.mustCall())));
50+
req.on('error',(err)=>{
51+
if(err.code!=='ECONNRESET')
52+
throwerr;
53+
});
4854
}));

‎test/parallel/test-http2-session-unref.js‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,23 @@ server.listen(0, common.mustCall(() => {
3434
// unref destroyed client
3535
{
3636
constclient=http2.connect(`http://localhost:${port}`);
37-
client.destroy();
38-
client.unref();
37+
38+
client.on('connect',common.mustCall(()=>{
39+
client.destroy();
40+
client.unref();
41+
}));
3942
}
4043

4144
// unref destroyed client
4245
{
4346
constclient=http2.connect(`http://localhost:${port}`,{
4447
createConnection: common.mustCall(()=>clientSide)
4548
});
46-
client.destroy();
47-
client.unref();
49+
50+
client.on('connect',common.mustCall(()=>{
51+
client.destroy();
52+
client.unref();
53+
}));
4854
}
4955
}));
5056
server.emit('connection',serverSide);

‎test/sequential/test-http2-max-session-memory.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ const largeBuffer = Buffer.alloc(1e6);
1313
constserver=http2.createServer({maxSessionMemory: 1});
1414

1515
server.on('stream',common.mustCall((stream)=>{
16+
stream.on('error',(err)=>{
17+
if(err.code!=='ECONNRESET')
18+
throwerr;
19+
});
1620
stream.respond();
1721
stream.end(largeBuffer);
1822
}));

0 commit comments

Comments
 (0)