nodejs / nodejs/node

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

Abierto
#66,003 2 comentarios 2 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Lenguaje dominante
JavaScript
Estrellas
122k
Forks
37.3k
Merge medio
4 d 2 h
PR fusionados (30 d)
283

Descripción

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.

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Empieza con src/quic/session.cc y la regresión existente en test/parallel/test-quic-datagram-multiple.mjs; lee el comportamiento documentado de maxDatagramSendAttempts y las referencias a ScheduleSessionFlush en src/quic/bindingdata.cc. Aplica los cambios indicados y después ejecuta ./configure --experimental-quic, make -j2 y python3 tools/test.py -j2 --timeout=30 parallel/test-quic-datagram-multiple.mjs. Se considera terminado cuando el burst retrasado de 32 datagramas recibe todos los paquetes sin abandono y las pruebas de datagramas existentes siguen pasando.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
cpp, javascript
Área
networking
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Activo
Claridad
Bien especificado
Aptitud para principiantes
38/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.