Skip to content

Conversation

Lukasa
Copy link
Collaborator

@Lukasa Lukasa commented Oct 14, 2025

Motivation

Right now, we insert HTTP/2 handlers in a callback on a future that is done very late. The result of this is that an entire ALPN negotiaton can complete before this callback is attached. That can in rare cases cause the HTTP/2 handler to miss the server preamble, because it gets added too late.

Modifications

This patch refactors the existing code to close that window. It does so by passing a promise into the connection path and completing that promise on the event loop where we add the ALPN handlers, which should ensure this will execute immediately when the ALPN negotiation completes. Immportantly, we attach our promise callbacks to that promise before we hand it off, making sure the timing windows go away.

Results

Timing window is closed

Motivation

Right now, we insert HTTP/2 handlers in a callback on a future that
is done very late. The result of this is that an entire ALPN negotiaton
_can_ complete before this callback is attached. That can in rare
cases cause the HTTP/2 handler to miss the server preamble, because
it gets added too late.

Modifications

This patch refactors the existing code to close that window. It does
so by passing a promise into the connection path and completing
that promise _on_ the event loop where we add the ALPN handlers, which
should ensure this will execute immediately when the ALPN negotiation
completes. Immportantly, we attach our promise callbacks to that
promise _before_ we hand it off, making sure the timing windows go
away.

Results

Timing window is closed
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Oct 14, 2025
@Lukasa Lukasa merged commit 0ce87cb into swift-server:main Oct 14, 2025
33 of 34 checks passed
@Lukasa Lukasa deleted the cb-alpn-yo-its-hard branch October 14, 2025 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants