-
Notifications
You must be signed in to change notification settings - Fork 30k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
http2: fix several timeout related issues
* correctly reset write timers: currently reset timers on both session & stream when write starts and when it ends. * prevent large writes from timing out: when writing a large chunk of data in http2, once the data is handed off to C++, the JS session & stream lose all track of the write and will timeout if the write doesn't complete within the timeout window Fix this issue by tracking whether a write request is ongoing and also tracking how many chunks have been sent since the most recent write started. (Since each write call resets the timer.) PR-URL: #16525 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]>
- Loading branch information
1 parent
c71c521
commit 857af41
Showing
6 changed files
with
274 additions
and
6 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| 'use strict'; | ||
| const common = require('../common'); | ||
| if (!common.hasCrypto) | ||
| common.skip('missing crypto'); | ||
| const assert = require('assert'); | ||
| const fixtures = require('../common/fixtures'); | ||
| const fs = require('fs'); | ||
| const http2 = require('http2'); | ||
| const path = require('path'); | ||
|
|
||
| common.refreshTmpDir(); | ||
|
|
||
| // This test assesses whether long-running writes can complete | ||
| // or timeout because the session or stream are not aware that the | ||
| // backing stream is still writing. | ||
| // To simulate a slow client, we write a really large chunk and | ||
| // then proceed through the following cycle: | ||
| // 1) Receive first 'data' event and record currently written size | ||
| // 2) Once we've read up to currently written size recorded above, | ||
| // we pause the stream and wait longer than the server timeout | ||
| // 3) Socket.prototype._onTimeout triggers and should confirm | ||
| // that the backing stream is still active and writing | ||
| // 4) Our timer fires, we resume the socket and start at 1) | ||
|
|
||
| const writeSize = 3000000; | ||
| const minReadSize = 500000; | ||
| const serverTimeout = common.platformTimeout(500); | ||
| let offsetTimeout = common.platformTimeout(100); | ||
| let didReceiveData = false; | ||
|
|
||
| const content = Buffer.alloc(writeSize, 0x44); | ||
| const filepath = path.join(common.tmpDir, 'http2-large-write.tmp'); | ||
| fs.writeFileSync(filepath, content, 'binary'); | ||
| const fd = fs.openSync(filepath, 'r'); | ||
|
|
||
| const server = http2.createSecureServer({ | ||
| key: fixtures.readKey('agent1-key.pem'), | ||
| cert: fixtures.readKey('agent1-cert.pem') | ||
| }); | ||
| server.on('stream', common.mustCall((stream) => { | ||
| stream.respondWithFD(fd, { | ||
| 'Content-Type': 'application/octet-stream', | ||
| 'Content-Length': content.length.toString(), | ||
| 'Vary': 'Accept-Encoding' | ||
| }); | ||
| stream.setTimeout(serverTimeout); | ||
| stream.on('timeout', () => { | ||
| assert.strictEqual(didReceiveData, false, 'Should not timeout'); | ||
| }); | ||
| stream.end(); | ||
| })); | ||
| server.setTimeout(serverTimeout); | ||
| server.on('timeout', () => { | ||
| assert.strictEqual(didReceiveData, false, 'Should not timeout'); | ||
| }); | ||
|
|
||
| server.listen(0, common.mustCall(() => { | ||
| const client = http2.connect(`https://localhost:${server.address().port}`, | ||
| { rejectUnauthorized: false }); | ||
|
|
||
| const req = client.request({ ':path': '/' }); | ||
| req.end(); | ||
|
|
||
| const resume = () => req.resume(); | ||
| let receivedBufferLength = 0; | ||
| let firstReceivedAt; | ||
| req.on('data', common.mustCallAtLeast((buf) => { | ||
| if (receivedBufferLength === 0) { | ||
| didReceiveData = false; | ||
| firstReceivedAt = Date.now(); | ||
| } | ||
| receivedBufferLength += buf.length; | ||
| if (receivedBufferLength >= minReadSize && | ||
| receivedBufferLength < writeSize) { | ||
| didReceiveData = true; | ||
| receivedBufferLength = 0; | ||
| req.pause(); | ||
| setTimeout( | ||
| resume, | ||
| serverTimeout + offsetTimeout - (Date.now() - firstReceivedAt) | ||
| ); | ||
| offsetTimeout = 0; | ||
| } | ||
| }, 1)); | ||
| req.on('end', common.mustCall(() => { | ||
| client.destroy(); | ||
| server.close(); | ||
| })); | ||
| })); |
Oops, something went wrong.