fix(quic): restore PTO CRYPTO retransmit lost in lock-split refactor
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
This commit is contained in:
+36
-5
@@ -154,12 +154,43 @@ class QuicConnectionDriver(
|
|||||||
Unit
|
Unit
|
||||||
}
|
}
|
||||||
if (woke == null) {
|
if (woke == null) {
|
||||||
// PTO fired. Set pendingPing so the writer emits a
|
// PTO fired. RFC 9002 §6.2.4: the probe packet MUST
|
||||||
// PING on the next drain (RFC 9002 §6.2.4 probe
|
// be ack-eliciting at the encryption level with
|
||||||
// packet). The peer's ACK feeds loss detection +
|
// unacknowledged data, and SHOULD retransmit lost
|
||||||
// retransmit (steps 5–6). Both fields are @Volatile;
|
// data rather than just emit a PING.
|
||||||
// no lock required.
|
//
|
||||||
|
// 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
|
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 =
|
||||||
(connection.consecutivePtoCount + 1).coerceAtMost(6)
|
(connection.consecutivePtoCount + 1).coerceAtMost(6)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user