nodejs / nodejs/node

quic: datagram bursts exhaust retry budget during zero-write loops

オープン
#66,003 コメント 2 件 リアクション 2 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

主要言語
JavaScript
スター
122k
フォーク
37.3k
平均マージ
4日 2時間
マージ済み PR(30日)
283

説明

Version

v26.8.2, built from source with ./configure --experimental-quic.

Source commit: f2f2c2f246c36bd74f082cb43ecfe830657d81c9.

Platform
Ubuntu 24.04.3 LTS
Linux 6.8.0 x86_64 GNU/Linux (uname -srmo)
GCC 13.3.0
Subsystem

node:quic — native datagram send queue in src/quic/session.cc.

What steps will reproduce the bug?

Send a finite synchronous burst of 32 datagrams, each 1080 bytes, through a loopback UDP relay that adds 20 ms per direction without intentionally dropping packets.

Save the following as repro.mjs in the root of a Node.js source checkout. Run it with a QUIC-enabled binary:

/path/to/node-with-quic --experimental-quic --no-warnings repro.mjs

The reproduction uses only built-in modules and the checkout's public test fixtures. It requires no browser, external server or npm dependencies. The client and server use separate local endpoints.

verifyPeer: 'manual' applies only to the public test certificate used in this loopback reproduction. The five-second timeout is a failure deadline.

// Run from a Node.js source checkout; fixture keys are public test fixtures.
import assert from 'node:assert/strict';
import { createSocket } from 'node:dgram';
import { createPrivateKey } from 'node:crypto';
import { readFileSync } from 'node:fs';
import { listen, connect } from 'node:quic';
const key = createPrivateKey(readFileSync('test/fixtures/keys/agent1-key.pem'));
const cert = readFileSync('test/fixtures/keys/agent1-cert.pem');
// A burst over a lossless link with 40ms RTT must survive pacing without
// exhausting the retry budget before the peer has had a chance to ACK.
{
  const burstCount = 32;
  const burstReceived = new Set();
  const burstDone = Promise.withResolvers();
  let abandonedCount = 0;
  let burstServerSession;
  const burstEndpoint = await listen((session) => {
    burstServerSession = session;
    session.ondatagram = () => {
      for (let index = 0; index < burstCount; index += 1) {
        const payload = new Uint8Array(1080).fill(index);
        session.sendDatagram(payload).catch(burstDone.reject);
      }
    };
  }, {
    sni: { '*': { keys: [key], certs: [cert] } },
    endpoint: { address: '127.0.0.1:0' },
    alpn: ['quic-test'],
    transportParams: { maxDatagramFrameSize: 1200 },
    ondatagramstatus: (identifier, status) => {
      if (status === 'abandoned') abandonedCount += 1;
    },
  });
  const relay = createSocket('udp4');
  const forwardingTimers = new Set();
  let clientPort;
  relay.on('message', (payload, remote) => {
    const fromServer = remote.port === burstEndpoint.address.port;
    if (!fromServer) clientPort = remote.port;
    const destinationPort = fromServer ? clientPort : burstEndpoint.address.port;
    // Deliberate network delay, not synchronization with a guessed sleep.
    const timer = setTimeout(() => {
      forwardingTimers.delete(timer);
      relay.send(payload, destinationPort, '127.0.0.1');
    }, 20);
    forwardingTimers.add(timer);
  });
  await new Promise(resolveBound => relay.bind(0, '127.0.0.1', resolveBound));
  const burstClient = await connect(`127.0.0.1:${relay.address().port}`, {
    endpoint: { address: '127.0.0.1:0' },
    alpn: 'quic-test',
    verifyPeer: 'manual',
    transportParams: { maxDatagramFrameSize: 1200 },
    ondatagram: (payload) => {
      assert.strictEqual(payload.length, 1080);
      assert.ok(payload.every(value => value === payload[0]));
      burstReceived.add(payload[0]);
      if (burstReceived.size === burstCount) burstDone.resolve();
    },
  });
  let burstTimer;
  try {
    await burstClient.opened;
    await burstClient.sendDatagram(Uint8Array.of(1));
    await Promise.race([burstDone.promise, new Promise((_, reject) => {
      burstTimer = setTimeout(() => reject(new Error(
        `Burst received ${burstReceived.size}/${burstCount}; abandoned=${abandonedCount}`)), 5000);
    })]);
    assert.strictEqual(abandonedCount, 0);
    console.log(JSON.stringify({ received: burstReceived.size, expected: burstCount, abandoned: abandonedCount }));
  } finally {
    clearTimeout(burstTimer);
    await Promise.all([burstClient.close(), burstServerSession?.close()]);
    await burstEndpoint.close();
    for (const timer of forwardingTimers) clearTimeout(timer);
    await new Promise(resolveClosed => relay.close(resolveClosed));
  }
}
How often does it reproduce? Is there a required condition?

The repository regression, which adds this burst scenario after the existing mixed stream/datagram case, failed in all three recorded runs with the original native send path.

Native send path Trial 1: received / abandoned Trial 2: received / abandoned Trial 3: received / abandoned
Original 23/32 / 9 26/32 / 6 24/32 / 8
Patched 32/32 / 0 32/32 / 0 32/32 / 0

The standalone reproduction above was also run independently once per binary. It received 24/32 datagrams with 8 abandoned on the original code, and 32/32 with zero abandoned on the patched code.

These results cover the finite burst and delayed loopback conditions described above; they do not establish a failure rate for other workloads.

What is the expected behavior? Why is that the expected behavior?

A queued datagram should not exhaust its send-attempt budget through repeated zero-write attempts in a tight native loop before ACK processing or pacing can allow progress.

The documented maxDatagramSendAttempts option limits the number of SendPendingData cycles. When a fresh datagram write returns zero, the session should yield until a later opportunity to send instead of immediately retrying with the same timestamp.

This report concerns premature local abandonment before transmission. It does not assume that QUIC DATAGRAM guarantees delivery.

What do you see instead?

The standalone reproduction fails with the original native send path:

Error: Burst received 24/32; abandoned=8

With the candidate patch, it completes successfully:

{"received":32,"expected":32,"abandoned":0}

Source inspection points to the following interaction:

  • SendPendingData enqueues/counts a zero-length stream write and can immediately repeat a fresh datagram attempt that returned zero.
  • TryWritePendingDatagram increments the attempt counter on zero.
  • Immediate flushing from SendDatagram allows a synchronous JS burst to consume attempts before ACK processing or pacing permits further progress.

The JavaScript binding and C++ SendDatagram both create SendPendingDataScope. These scopes share a depth counter; they are not two independent flushes per call.

Additional information

Candidate fix

The patch and recorded validation results are attached here:

nodejs-upstream-review-20260912.zip

The archive contains patch-nodequic-datagram-upstream.patch, which modifies only:

  • src/quic/session.cc
  • test/parallel/test-quic-datagram-multiple.mjs

The proposed changes:

  1. Enqueue/count packets only when their length is positive.
  2. Return after a zero-result fresh datagram attempt, allowing a later ACK/pacing opportunity.
  3. Use the existing ScheduleSessionFlush mechanism to flush the JS burst after the current callback returns, removing both immediate send scopes.
  4. Extend the existing regression test with the delayed loopback burst and an explicitly separate client endpoint.

The patch preserves NGTCP2_ERR_WRITE_MORE handling, queue capacity, drop policy and configured retry limits.

To apply and test from a clean source checkout:

git apply --check /path/to/patch-nodequic-datagram-upstream.patch
git apply /path/to/patch-nodequic-datagram-upstream.patch
./configure --experimental-quic
make -j2
python3 tools/test.py -j2 --timeout=30 parallel/test-quic-datagram-multiple.mjs

Validation and scope

  • The baseline lab binary contains unrelated HTTP/3 SETTINGS additions. Its native datagram send path matches v26.8.2, and the reproduction uses raw quic-test ALPN, bypassing HTTP/3.
  • 44 HTTP/3, datagram and preferred-address tests passed with the native fix, including drop-policy, stream-idle-timeout and 0-RTT cases.
  • The attached patch has the same executable C++ changes as that tested fix. A comment was corrected and explicit client-endpoint isolation was added to the regression test; the revised test also passed.
  • The patch applies to v26.8.2 and upstream commit 224caba540129389d10297a05dd646fb3c5f489b. Five selected send-path function bodies match v26.8.2 byte for byte at that upstream commit. That commit was compared statically, not rebuilt or runtime-tested.
  • Full cross-platform CI, sanitizer testing and long-duration soak testing have not been performed.

Related discussion and references

#64422 discusses related callback/scheduling concerns, but it has not been established as the same issue as this finite-burst reproduction.

AI assistance

GLM and Codex assisted with the investigation, candidate patch, automated validation and editing of this report.

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

src/quic/session.cc と test/parallel/test-quic-datagram-multiple.mjs にある既存のリグレッションから始めます。maxDatagramSendAttempts の文書化された動作と、src/quic/bindingdata.cc にある ScheduleSessionFlush への参照を確認します。報告された変更を適用し、その後 ./configure --experimental-quic、make -j2、python3 tools/test.py -j2 --timeout=30 parallel/test-quic-datagram-multiple.mjs を実行します。遅延した 32 個のデータグラムのバーストが破棄されることなくすべてのパケットを受信し、既存のデータグラムテストが成功すれば完了です。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
cpp, javascript
領域
networking
issue の種類
バグ
難易度
4/5
見積もり時間
3〜5日
活発さ
活発
明瞭さ
明確に書かれている
初心者へのやさしさ
38/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。