Skip to content

Only poll ApplicationOverQuic wait_for_data after handshake is ready or we're in early data#2480

Open
MaxF12 wants to merge 1 commit into
cloudflare:masterfrom
MaxF12:master
Open

Only poll ApplicationOverQuic wait_for_data after handshake is ready or we're in early data#2480
MaxF12 wants to merge 1 commit into
cloudflare:masterfrom
MaxF12:master

Conversation

@MaxF12

@MaxF12 MaxF12 commented May 15, 2026

Copy link
Copy Markdown

tokio-quiche can poll wait_for_data() too early during connection setup. For apps that always report should_act()==true an overly eager wait_for_data() could preempt the handshake which might stall the handshake itself. Now wait_for_data() can only be polled if the 1-RTT handshake is either completed or we are in early-data

@MaxF12
MaxF12 requested a review from a team as a code owner May 15, 2026 00:30
@MaxF12 MaxF12 changed the title Only poll ApplicationOverQuic wait_for_data after handshake is ready Only poll ApplicationOverQuic wait_for_data after handshake is ready or we're in early data May 15, 2026
if quic_application.should_act() {
let connection_ready = qconn.is_established() || qconn.is_in_early_data();

if connection_ready && quic_application.should_act() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes some sense to me. What tests could we add for this case to ensure we don't regress?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added regression tests now

Prevent eager application wakeups from preempting handshake progress before the connection is established. Keep handshake driving in the quiche path until 1-RTT or early-data readiness.\n\nAdd a regression test using an eager application and asynchronous certificate callback to ensure the handshake reaches on_conn_established().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants