diff --git a/src/quic/application.cc b/src/quic/application.cc index 688a1309dea552..109771e0f58bac 100644 --- a/src/quic/application.cc +++ b/src/quic/application.cc @@ -425,6 +425,7 @@ class DefaultApplication final : public Session::Application { // The peer granted more flow control for this stream. Re-schedule // it so SendPendingData will resume writing. DCHECK_NOT_NULL(stream); + stream->UpdateWriteDesiredSize(); // the stream might be blocked on js side stream->Schedule(&stream_queue_); } diff --git a/src/quic/http3.cc b/src/quic/http3.cc index efaf1959427058..ba4b5c9eb0ffec 100644 --- a/src/quic/http3.cc +++ b/src/quic/http3.cc @@ -375,6 +375,7 @@ class Http3ApplicationImpl final : public Session::Application { Debug(&session(), "HTTP/3 application extending max stream data to %" PRIu64, max_data); + stream->UpdateWriteDesiredSize(); // the stream might be blocked on js side nghttp3_conn_unblock_stream(*this, stream->id()); } diff --git a/test/parallel/test-quic-h3-maxstreamdata-external-buffer-failure.mjs b/test/parallel/test-quic-h3-maxstreamdata-external-buffer-failure.mjs new file mode 100644 index 00000000000000..df74e4db408b06 --- /dev/null +++ b/test/parallel/test-quic-h3-maxstreamdata-external-buffer-failure.mjs @@ -0,0 +1,80 @@ +// Flags: --experimental-quic --experimental-stream-iter --no-warnings + +// Test: Quic maxstreamdata updates on http/3 +// Client sends a body that precisely fills the 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: HTTP/3 spends 11 of those bytes on framing (8 for +// the HEADERS frame below, 3 for the DATA frame header). The send buffer then +// empties at the same moment the window reaches zero, leaving nothing in +// flight to ack. Any other size leaves bytes queued, and the ack for those +// wakes the writer instead, hiding the bug. +const BODY = WINDOW - 11; + +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: WINDOW, + initialMaxData: 1024 * 1024, + }, + 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); diff --git a/test/parallel/test-quic-maxstreamdata-external-buffer-failure.mjs b/test/parallel/test-quic-maxstreamdata-external-buffer-failure.mjs new file mode 100644 index 00000000000000..79d11e05794a0b --- /dev/null +++ b/test/parallel/test-quic-maxstreamdata-external-buffer-failure.mjs @@ -0,0 +1,75 @@ +// Flags: --experimental-quic --experimental-stream-iter --no-warnings + +// Test: Quic maxstreamdata updates on pure quic +// Client sends a body that precisely fills the 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: HTTP/3 spends 11 of those bytes on framing (8 for +// the HEADERS frame below, 3 for the DATA frame header). The send buffer then +// empties at the same moment the window reaches zero, leaving nothing in +// flight to ack. Any other size leaves bytes queued, and the ack for those +// wakes the writer instead, hiding the bug. +const BODY = WINDOW; + +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 */ } + }; +}, { + alpn: 'foo', + sni: { '*': { keys: [key], certs: [cert] } }, + transportParams: { + initialMaxStreamDataBidiRemote: WINDOW, + initialMaxData: 1024 * 1024, + } +}); + +const session = await connect(endpoint.address, { + servername: 'localhost', + verifyPeer: 'manual', + alpn: 'foo' +}); +await session.opened; + +// Budget well above the window, so the window is what stops the writer. +const stream = await session.createBidirectionalStream({ budget: 1024 * 1024 }); + +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);