nodejs / nodejs/node

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

Đang mở
#66,003 2 bình luận 2 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

Ngôn ngữ chính
JavaScript
Star
122k
Fork
37.3k
Merge trung bình
4 ngày 2 giờ
Pull request đã merge (30 ngày)
283

Mô tả

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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu với src/quic/session.cc và regression hiện có trong test/parallel/test-quic-datagram-multiple.mjs; đọc hành vi đã được ghi lại của maxDatagramSendAttempts và các tham chiếu đến ScheduleSessionFlush trong src/quic/bindingdata.cc. Áp dụng các thay đổi được báo cáo, sau đó chạy ./configure --experimental-quic, make -j2 và python3 tools/test.py -j2 --timeout=30 parallel/test-quic-datagram-multiple.mjs. Hoàn tất có nghĩa là burst gồm 32 datagram bị trì hoãn nhận được tất cả các packet mà không bị từ bỏ, đồng thời các bài kiểm thử datagram hiện có đều vượt qua.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
cpp, javascript
Lĩnh vực
networking
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
38/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.