Conversation
Fixes four doc-comment findings on Task 2's connection.rs: connection_clone now points at watch.rs's path watchers (the last strict caller, since the four stream openers moved to connection_clone_any) and names why paths_stream and path_events require HandshakeCompleted; the outgoing 0-RTT free doc now describes handshake_completed's actual ZeroRttStatus-wrapped return instead of the incoming side's bare Connection; peek's Safety clause now says a differently-tagged handle is a supported None rather than a violation, which is what connection_clone_any's three-way probe relies on; and with_any's doc now calls the Arc clone a refcount bump instead of claiming it costs no more than a borrow.
…ed ticket wait handleOrZero was a byte-for-byte copy of handleOrThrow built on a false premise (handleOrThrow never raised on a null handle); it's gone, and handleOrThrow now just reads the first half of handleAndAcceptedOrThrow, so one function owns the unsafe pointer teardown. The priming dial in the second-dial 0-RTT test no longer sleeps a fixed 300ms hoping the session ticket has landed — it retries dialling until zeroRtt() itself reports one, bounded by the suite's real-time timeout. Along the way, exercising OutgoingZeroRttConnection.remoteId() for the first time surfaced a real bug in ffi.kt's use(): its null check couldn't tell "the result pointer was null" from "bytesOrNull legitimately answered null", so the absent case crashed instead of returning null. Fixed by checking only the receiver. Also: bytesOrNull now floors a negative bytes_len instead of trusting it, two KDoc comments no longer link symbols that don't exist until later tasks, zero_rtt_clone is renamed to peek_clone with connection_clone delegating to it, and the three Connecting handles the 0-RTT dialling tests left unreleased now close like every other startConnect site in the file.
…dinal in Rust The restart-rejection test discarded the Connecting from startConnect() after zeroRtt() spent it, leaking a connection-domain NativeHandle for the life of the process - the same handle the priming loop two blocks above already knows needs its own close. core.rs also gets a compile-time assert pinning ERROR_ZERO_RTT_REJECTED to 15, so a mid-list insertion that renumbers it away from the ordinal IrohError.kt pins fails the build here instead of silently remapping every error in the binding.
Four places claimed maxTlsTickets = 0 "stores none" and therefore effectively disables 0-RTT. Traced against rustls 0.23.42 and confirmed against the real library on both facades: for an endpoint that only ever dials one peer, it does not. LimitedCache::new(0) rounds down to a per-server budget of 0, but VecDeque::with_capacity(0)'s first push_back reallocates past that (to capacity 4, not 1), so the cache never evicts the one entry a single-peer endpoint ever creates and 0-RTT works exactly as it would with no limit set — measured 2 attempts to a ticket, 3/3 runs, on both jvmTest and macosArm64Test. Adds `maxTlsTickets 0 does not stop a single peer from resuming` to CommonConnectionTests (delegated to native/jvm/androidHostTest) and corrects the four sites that made the false claim: Endpoint.kt's KDoc, endpoint.rs's configure(), STATUS.md, and the zero-rtt design spec. Also fixes a fifth, smaller repetition of the same claim in a test comment, and updates the test-count bookkeeping in AGENTS.md/README.md/STATUS.md for the one new shared body (239 -> 240 per facade).
Gates: cargo test runs in CI now (lint job), alongside AGENTS.md's gate list — nothing previously typechecked stream.rs's #[cfg(test)] module, the substitute for the write-based ZeroRttRejected assertion this branch could not make. Documentation and comments, one per finding: - RecvStream.is0Rtt()'s KDoc was flatly wrong for a server-opened stream: poll_open only sets the flag on the dialling side, so IncomingZeroRttConnection.openBi() always reports false. Reconciled with the Rust-side doc near recv_is_0rtt and retargeted a stale Connection.setMaxConcurrentBiStreams link to QuicConnection. - Four stale/wrong comments in connection.rs: two "Task N" references to an untracked plan, one non-existent Incoming::into_0rtt corrected to Accepting::into_0rtt, and two comments that described unimplemented work that has since landed. - AGENTS.md's Handles section now carries the same caveat connection.rs documents: connecting_zero_rtt puts a Connecting back on the no-ticket branch, so a second use is Some again rather than ERROR_CLOSED. - The is0Rtt() unit test now reads a byte and pins bytesRead() == 1 first, so an export wired to the wrong accessor could not pass by accident. - STATUS.md and the design spec now give both reasons is0Rtt() == true is never asserted (poll_accept's sampling point on the accepting side, the three-independently-scheduled-tokio-task race on the dialling side), not just the accepting-side half. - README.md's unparseable "binary compatible with source" sentence fixed to "source-compatible, not binary-compatible". - The leak test's comment now says what its counter actually catches (a missing handle release) rather than a connection-lifetime claim it can't back up. - The cancelled-await test now enforces its own premise: cancelAndJoin on an already-completed Deferred is a no-op, so the body now awaits the cancelled Deferred and fails loudly (after closing any handle it produced) if it turns out not to have been cancelled after all. - Connecting.zeroRtt()'s KDoc now documents that a cancelled call can also consume the attempt, not just a non-null answer. - read_max_tls_tickets's i64 read moves from ERROR_INVALID_ARGUMENT to ERROR_BIND, matching the presence byte read next to it: a truncated payload is a malformed message, not a bad argument value. Also fixed two wrong noq-1.1.0/src/connection.rs line citations (:1094 -> :1098) found while correcting the adjacent comments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds 0-RTT on both the dialling and the accepting side, so an application that reconnects to a peer it has spoken to before can send application data in the first flight instead of paying a full handshake round trip.
The shape, and why it is not a flag
Upstream models a 0-RTT connection as a distinct Rust type with a smaller method surface: no paths, and
alpn()/remote_id()become optional. That collides with this binding'sTypeId-tagged handles, withstream.rs's typing, and with the path watchers.Rather than weaken
Connection's contract for everyone — itsalpn()KDoc promises "always present" — the surface every handshake state shares moved into asealed class QuicConnection.Connectionkeeps what only a completed handshake has (alpn,remoteId,side,paths,rtt);OutgoingZeroRttConnectionandIncomingZeroRttConnectionadd nullablealpn/remoteId.Stream.kt's four openers moved to the supertype, because sending early data is opening a stream.On the Rust side the shared surface gained no new exports:
connection_clone_anyprobes the three tags in turn and forwards through anAnyConnectionenum, so the existing exports accept any handshake state. The strictconnection_clonestays forwatch.rs's path watchers and the four handshake-completed-only readers.API
Connecting.zeroRtt()answersnullrather than raising when the endpoint holds no ticket, and the attempt stays usable — that is upstream's two-way branch, not a failure.ZeroRttStatus.Rejectedcarries a working connection: rejection means the early streams are dead and their data must be resent.Also:
EndpointConfig.maxTlsTickets,RecvStream.is0Rtt(), andIrohError.Code.ZeroRttRejectedso a refused early stream is distinguishable from an ordinary write failure.What is proven, and what is not
Acceptance and rejection are both demonstrated over loopback, rejection by restarting the server endpoint under the same
SecretKey— upstream's own recipe.Two things are deliberately not asserted, both measured rather than assumed, and both written into
STATUS.md:ZeroRttRejectedcode is not pinned by a live refused write. It was tried: 0 of 9 runs on the JVM facade, 10 of 16 on cinterop. noq captures a stream'sis_0rttonce, at the open's first poll, and this binding dispatcheszeroRtt,openBiandawaitHandshakeas three independently spawned tokio tasks — so a driver that already processed the rejection leaves an ordinary stream whose write correctly succeeds. It is pinned instead by Rust unit tests over the four failure mappers, a Kotlin ordinal assertion, and a compile-time assertion that the constant is 15.is0Rtt() == trueis asserted nowhere:poll_acceptsamples the handshake state when the stream is accepted rather than when it was created.Corrections this work forced elsewhere
maxTlsTickets = 0does not disable 0-RTT, though the design said it did.rustlsrounds it to a per-server budget of 0, butVecDeque::with_capacity(0)'s firstpush_backovershoots to capacity 4, so the eviction condition never fires and the ticket is kept. Measured, not read: a ticket arrives in two dial attempts, 3/3 runs, on both facades. The claim is retracted in the KDoc, in Rust and inSTATUS.md, and a test now pins the real behaviour.cargo testran in no gate. The crate's first#[cfg(test)]module arrived here, and clippy without--all-targetsdoes not even typecheck one — so the substitute pinning above would have been dead weight in CI.cargo testis now inAGENTS.md's gate list and inci.yml.Once's "exactly one caller" invariant now has one documented exception, andAGENTS.mdsays so alongsideconnection.rs.Compatibility
Source-compatible. Not binary-compatible:
Stream.kt's four extension functions changed their first parameter type fromConnectiontoQuicConnection, so consumers compiled against an earlier build must recompile.Verification
240 shared bodies per tested facade (480 across
jvmTestandmacosArm64Test), 246 on Android, 726 host tests, plus 4 Rust unit tests. All six gates green.