Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Back out new typed HTTP protocol upgrader (#2579)" #2593

Merged
merged 2 commits into from
Nov 15, 2023

Conversation

FranzBusch
Copy link
Member

Motivation

We have reverted the typed HTTP protocol upgrader pieces since adopters were running into a compiler bug (swiftlang/swift#69459) that caused the compiler to emit strong references to swift_getExtendedExistentialTypeMetadata. The problem is that swift_getExtendedExistentialTypeMetadata is not available on older runtimes before constrained existentials have been introduced. This caused adopters to run into runtime crashes when loading any library compiled with this NIO code.

Modifications

This PR reverts the revert and guard all new code in a compiler guard that checks that we are either on non-Darwin platforms or on a new enough Swift compiler that contains the fix.

Result

We can offer the typed HTTP upgrade code to our adopters again.

@FranzBusch
Copy link
Member Author

@glbrntt The only changes in here are adapting to the new NIOAsyncChannel.executeThenClose APIs and adding this compiler guard #if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10)) in a few places

@FranzBusch
Copy link
Member Author

@swift-server-bot test this please

@glbrntt
Copy link
Contributor

glbrntt commented Nov 15, 2023

@FranzBusch is it much bother for you to split this into two commits? I.e. the revert (even if it doesn't build) then the fixups? Much easier to review then...

# Motivation
We have reverted the typed HTTP protocol upgrader pieces since adopters were running into a compiler bug (swiftlang/swift#69459) that caused the compiler to emit strong references to `swift_getExtendedExistentialTypeMetadata`. The problem is that `swift_getExtendedExistentialTypeMetadata` is not available on older runtimes before constrained existentials have been introduced. This caused adopters to run into runtime crashes when loading any library compiled with this NIO code.

# Modifications
This PR reverts the revert and guard all new code in a compiler guard that checks that we are either on non-Darwin platforms or on a new enough Swift compiler that contains the fix.

# Result
We can offer the typed HTTP upgrade code to our adopters again.
@FranzBusch
Copy link
Member Author

@glbrntt This is now separated in the revert and the fix-ups

@FranzBusch FranzBusch merged commit c461993 into apple:main Nov 15, 2023
8 checks passed
@FranzBusch FranzBusch deleted the fb-revert-typed-handler branch November 15, 2023 18:58
@FranzBusch FranzBusch added the 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 label Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants