quic: buffer stalls, as maxdata updates do not triggern an update of writeDesired sizes
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 122k
- Forks
- 37.3k
- Avg merge
- 4d 2h
- Merged PRs (30d)
- 283
Description
ngtcp2 has no callback to inform us about an arrived maxdata frame.
If the buffering is only external and not within ngtcp2, this can cause a stall.
The equivalent for maxstreamdata is already addressed in https://github.com/nodejs/node/pull/64768 , as here ngtcp2 provides a callback.
Here is a reproduction code, based on the test of @pimterry for the PR of maxstreamdata:
// Flags: --experimental-quic --experimental-stream-iter --no-warnings
// Test: Quic maxdata updates on http/3
// Client sends a body that precisely fills the session window size,
// and verifies that it is data transfer is not stalled.
import { hasQuic, skip } from '../common/index.mjs';
import { readFile } from 'node:fs/promises';
import { setTimeout as sleep } from 'node:timers/promises';
if (!hasQuic) {
skip('QUIC is not enabled');
}
const { listen, connect } = await import('node:quic');
const { createPrivateKey } = await import('node:crypto');
const { drainableProtocol } = await import('stream/iter');
const keys = 'test/fixtures/keys';
const key = createPrivateKey(await readFile(`${keys}/agent1-key.pem`));
const cert = await readFile(`${keys}/agent1-cert.pem`);
const WINDOW = 4096;
// Fills the window exactly:
// considers all framing including some initial session capsules
const BODY = WINDOW - 38;
let letServerRead;
const serverMayRead = new Promise((resolve) => { letServerRead = resolve; });
const endpoint = await listen((session) => {
session.onstream = async (stream) => {
await serverMayRead;
// eslint-disable-next-line no-unused-vars
for await (const _ of stream) { /* reading extends the window */ }
};
}, {
sni: { '*': { keys: [key], certs: [cert] } },
transportParams: {
initialMaxStreamDataBidiRemote: 1024 * 1024, // make sure maxstreamdata does not block
initialMaxData: WINDOW,
},
onheaders() { this.sendHeaders({ ':status': '200' }); },
});
const session = await connect(endpoint.address, {
servername: 'localhost',
verifyPeer: 'manual',
});
await session.opened;
// Budget well above the window, so the window is what stops the writer.
const stream = await session.createBidirectionalStream({ budget: 1024 * 1024 });
stream.sendHeaders({
':method': 'POST',
':path': '/',
':scheme': 'https',
':authority': 'localhost',
}, { terminal: false });
const writer = stream.writer;
writer.writeSync(new Uint8Array(BODY));
// Long enough for every byte to be acked. The peer acks as data arrives,
// whether or not its application has read any of it, so by now the window is
// exhausted, the send buffer is empty, and no further ACK can arrive.
await sleep(500);
const watchdog = setTimeout(() => {
console.error('STALLED: no drain after MAX_STREAM_DATA');
process.exit(1);
}, 5000);
letServerRead(); // Extend the window, with no ack attached
await writer[drainableProtocol]();
clearTimeout(watchdog);
process.exit(0);
This is the ngtcp2 issue:
https://github.com/ngtcp2/ngtcp2/issues/2243 .
Or is there another way without a ngtcp2 callback? (@pimterry @jasnell )
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by running the supplied QUIC reproduction and inspect the write/drain path used by the session and stream writer. Compare the maxdata behavior with the maxstreamdata fix in pull request 64768 and review ngtcp2 issue 2243; done means the writer drain completes after the server extends the session window without stalling.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100