-
Notifications
You must be signed in to change notification settings - Fork 653
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
Fix the HappyEyeballsResolver and core Bootstraps under strict concurrency #3062
Fix the HappyEyeballsResolver and core Bootstraps under strict concurrency #3062
Conversation
These two changes are tied together: the HER severely affects the bootstraps setup logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks Cory
Ok, I'm going to take the L on this extra allocation. I'm pretty sure this is the result of needing to no longer close over |
channelInitializer: channelInitializer, | ||
channelOptions: channelOptions, | ||
bindTarget: bindTarget | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifically I think this is our new allocation.
Motivation:
The HappyEyeballsResolver, being an old part of our stack, has a lot of code in it that fails to pass strict concurrency checking. That's deeply suboptimal.
Modifications:
Result:
Another step taken on the road to strict concurrency.