From cf2303a38d555b8ae2257887632711127a0b2c79 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 7 May 2026 02:46:55 +0000 Subject: [PATCH] fix(quic): restore PTO CRYPTO retransmit lost in lock-split refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit aioquic interop multiplexing qlog (the smoking gun): packet_sent PN=0 Initial frames=[crypto] (ClientHello) packet_sent PN=1 Initial frames=[ping] (PTO probe — bare PING) packet_received connection_close 0x0 "Packet contains no CRYPTO frame" This is the SAME bug commit c0d7b6031 fixed; the lock-split refactor (ef4bb9998) re-wrote the driver's PTO branch to use streamsLock and inlined the @Volatile pendingPing/consecutivePtoCount writes — but dropped the requeueAllInflightCrypto call that c0d7b6031 had wired under the (now-renamed) connection.lock. Restored under levelLock for the appropriate level. SendBuffer's internal synchronized covers correctness, but the level lock keeps us from racing the writer's takeChunk mid-build. The writer's comment at QuicConnectionWriter.kt:421-431 already documented this contract — it was just unwired on the driver side. https://claude.ai/code/session_01HcvfQq1ttPV9PkRoJb4nyT --- .../quic/connection/QuicConnectionDriver.kt | 41 ++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt index 0864326e1..23c1f792f 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt @@ -154,12 +154,43 @@ class QuicConnectionDriver( Unit } if (woke == null) { - // PTO fired. Set pendingPing so the writer emits a - // PING on the next drain (RFC 9002 §6.2.4 probe - // packet). The peer's ACK feeds loss detection + - // retransmit (steps 5–6). Both fields are @Volatile; - // no lock required. + // PTO fired. RFC 9002 §6.2.4: the probe packet MUST + // be ack-eliciting at the encryption level with + // unacknowledged data, and SHOULD retransmit lost + // data rather than just emit a PING. + // + // Pre-1-RTT we have a concrete thing to retransmit: + // the unacknowledged ClientHello (at Initial) or + // ClientFinished (at Handshake). Requeue ALL + // currently-inflight CRYPTO bytes for the highest + // active pre-application level so the next drain's + // takeChunk emits a fresh CRYPTO frame at the + // original offset. pendingPing stays set as a + // fallback only — `collectHandshakeLevelFrames` + // skips the PING when CRYPTO is in the same frame + // list. aioquic strictly rejects pre-handshake + // Initials with no CRYPTO frame ("Packet contains + // no CRYPTO frame"), so a bare-PING probe is fatal. + // + // Post-handshake (1-RTT installed) we keep the + // bare-PING behavior; STREAM retransmit is driven + // by ACK-driven packet-threshold loss detection. + // + // pendingPing + consecutivePtoCount are @Volatile. + // requeueAllInflightCrypto mutates the level's + // cryptoSend buffer; the writer reads it under + // levelLock during drainOutbound, so we take that + // lock for the requeue to avoid racing the writer's + // takeChunk with our requeue mid-build. connection.pendingPing = true + if (connection.application.sendProtection == null) { + val level = highestPreApplicationLevel(connection) + if (level != null) { + connection.levelState(level).levelLock.withLock { + connection.requeueAllInflightCrypto(level) + } + } + } connection.consecutivePtoCount = (connection.consecutivePtoCount + 1).coerceAtMost(6) }