Skip to content

Commit d000fbf

Browse files
fix(http2): fix possible race condition when handling http2 stream on almost timed out session by improving timeout out algorithm; (#7186)
1 parent 08db960 commit d000fbf

2 files changed

Lines changed: 62 additions & 21 deletions

File tree

lib/adapters/http.js

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,15 @@ class Http2Sessions {
8787

8888
const session = connect(authority, options);
8989

90-
session.once('close', () => {
90+
let removed;
91+
92+
const removeSession = () => {
93+
if (removed) {
94+
return;
95+
}
96+
97+
removed = true;
98+
9199
let entries = authoritySessions, len = entries.length, i = len;
92100

93101
while (i--) {
@@ -99,9 +107,41 @@ class Http2Sessions {
99107
}
100108
}
101109
}
102-
});
110+
};
111+
112+
const originalRequestFn = session.request;
113+
114+
const {sessionTimeout} = options;
115+
116+
if(sessionTimeout != null) {
117+
118+
let timer;
119+
let streamsCount = 0;
120+
121+
session.request = function () {
122+
const stream = originalRequestFn.apply(this, arguments);
123+
124+
streamsCount++;
103125

104-
Http2Sessions.setTimeout(session, options.sessionTimeout);
126+
if (timer) {
127+
clearTimeout(timer);
128+
timer = null;
129+
}
130+
131+
stream.once('close', () => {
132+
if (!--streamsCount) {
133+
timer = setTimeout(() => {
134+
timer = null;
135+
removeSession();
136+
}, sessionTimeout);
137+
}
138+
});
139+
140+
return stream;
141+
}
142+
}
143+
144+
session.once('close', removeSession);
105145

106146
let entries = this.sessions[authority], entry = [
107147
session,
@@ -112,12 +152,6 @@ class Http2Sessions {
112152

113153
return session;
114154
}
115-
116-
static setTimeout(session, timeout = 1000) {
117-
session && session.setTimeout(timeout, () => {
118-
session.close();
119-
});
120-
}
121155
}
122156

123157
const http2Sessions = new Http2Sessions();
@@ -284,10 +318,12 @@ export default isHttpAdapterSupported && function httpAdapter(config) {
284318
let rejected = false;
285319
let req;
286320

287-
httpVersion = Number(httpVersion);
321+
httpVersion = +httpVersion;
322+
288323
if (Number.isNaN(httpVersion)) {
289324
throw TypeError(`Invalid protocol version: '${config.httpVersion}' is not a number`);
290325
}
326+
291327
if (httpVersion !== 1 && httpVersion !== 2) {
292328
throw TypeError(`Unsupported protocol version '${httpVersion}'`);
293329
}

test/unit/adapters/http.js

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2423,7 +2423,7 @@ describe('supports http with nodejs', function () {
24232423
});
24242424

24252425
it('should support request cancellation', async function (){
2426-
if (typeof AbortSignal !== 'function') {
2426+
if (typeof AbortSignal !== 'function' || !AbortSignal.timeout) {
24272427
this.skip();
24282428
}
24292429

@@ -2525,13 +2525,17 @@ describe('supports http with nodejs', function () {
25252525

25262526
it("should use different sessions for different authorities", async() => {
25272527
server = await startHTTPServer((req, res) => {
2528-
setTimeout(() => res.end('OK'), 1000);
2528+
setTimeout(() => {
2529+
res.end('OK');
2530+
}, 2000);
25292531
}, {
25302532
useHTTP2: true
25312533
});
25322534

25332535
server2 = await startHTTPServer((req, res) => {
2534-
setTimeout(() => res.end('OK'), 1000);
2536+
setTimeout(() => {
2537+
res.end('OK');
2538+
}, 2000);
25352539
}, {
25362540
useHTTP2: true,
25372541
port: SERVER_PORT2
@@ -2559,7 +2563,9 @@ describe('supports http with nodejs', function () {
25592563

25602564
it("should use different sessions for requests with different http2Options set", async() => {
25612565
server = await startHTTPServer((req, res) => {
2562-
setTimeout(() => res.end('OK'), 1000);
2566+
setTimeout(() => {
2567+
res.end('OK')
2568+
}, 1000);
25632569
}, {
25642570
useHTTP2: true
25652571
});
@@ -2639,6 +2645,8 @@ describe('supports http with nodejs', function () {
26392645
}
26402646
});
26412647

2648+
const data1 = await getStream(response1.data);
2649+
26422650
await setTimeoutAsync(5000);
26432651

26442652
const response2 = await http2Axios.get(LOCAL_SERVER_URL, {
@@ -2648,15 +2656,12 @@ describe('supports http with nodejs', function () {
26482656
}
26492657
});
26502658

2659+
const data2 = await getStream(response2.data);
2660+
26512661
assert.notStrictEqual(response1.data.session, response2.data.session);
26522662

2653-
assert.deepStrictEqual(
2654-
await Promise.all([
2655-
getStream(response1.data),
2656-
getStream(response2.data)
2657-
]),
2658-
['OK', 'OK']
2659-
);
2663+
assert.strictEqual(data1, 'OK');
2664+
assert.strictEqual(data2, 'OK');
26602665
});
26612666
});
26622667
});

0 commit comments

Comments
 (0)