Skip to content

Commit 559a8e3

Browse files
addaleaxBethGriggs
authored andcommittedSep 25, 2019
http2: do not crash on stream listener removal w/ destroyed session
Do not crash when the session is no longer available. Fixes: #29457 PR-URL: #29459 Backport-PR-URL: #29618 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 92a2f8b commit 559a8e3

File tree

2 files changed

+39
-4
lines changed

2 files changed

+39
-4
lines changed
 

‎lib/internal/http2/core.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -363,23 +363,27 @@ function sessionListenerRemoved(name) {
363363
// Also keep track of listeners for the Http2Stream instances, as some events
364364
// are emitted on those objects.
365365
function streamListenerAdded(name) {
366+
const session = this[kSession];
367+
if (!session) return;
366368
switch (name) {
367369
case 'priority':
368-
this[kSession][kNativeFields][kSessionPriorityListenerCount]++;
370+
session[kNativeFields][kSessionPriorityListenerCount]++;
369371
break;
370372
case 'frameError':
371-
this[kSession][kNativeFields][kSessionFrameErrorListenerCount]++;
373+
session[kNativeFields][kSessionFrameErrorListenerCount]++;
372374
break;
373375
}
374376
}
375377

376378
function streamListenerRemoved(name) {
379+
const session = this[kSession];
380+
if (!session) return;
377381
switch (name) {
378382
case 'priority':
379-
this[kSession][kNativeFields][kSessionPriorityListenerCount]--;
383+
session[kNativeFields][kSessionPriorityListenerCount]--;
380384
break;
381385
case 'frameError':
382-
this[kSession][kNativeFields][kSessionFrameErrorListenerCount]--;
386+
session[kNativeFields][kSessionFrameErrorListenerCount]--;
383387
break;
384388
}
385389
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const http2 = require('http2');
7+
8+
// Regression test for https://github.com/nodejs/node/issues/29457:
9+
// HTTP/2 stream event listeners can be added and removed after the
10+
// session has been destroyed.
11+
12+
const server = http2.createServer((req, res) => {
13+
res.end('Hi!\n');
14+
});
15+
16+
server.listen(0, common.mustCall(() => {
17+
const client = http2.connect(`http://localhost:${server.address().port}`);
18+
const headers = { ':path': '/' };
19+
const req = client.request(headers);
20+
21+
req.on('close', common.mustCall(() => {
22+
req.removeAllListeners();
23+
req.on('priority', common.mustNotCall());
24+
server.close();
25+
}));
26+
27+
req.on('priority', common.mustNotCall());
28+
req.on('error', common.mustCall());
29+
30+
client.destroy();
31+
}));

0 commit comments

Comments
 (0)
Please sign in to comment.