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

HttpClient: Timing issue in sync ConnectHelper.Connect #40876

Closed
geoffkizer opened this issue Aug 15, 2020 · 2 comments · Fixed by #40880
Closed

HttpClient: Timing issue in sync ConnectHelper.Connect #40876

geoffkizer opened this issue Aug 15, 2020 · 2 comments · Fixed by #40880

Comments

@geoffkizer
Copy link
Contributor

This code has a timing issue:

                using (cancellationToken.UnsafeRegister(static s => ((Socket)s!).Dispose(), socket))
                {
                    socket.Connect(new DnsEndPoint(host, port));
                }

If the socket.Connect call succeeds, but then the CancellationToken fires before the registration is disposed, then the Socket will be disposed but we will still continue as if the Connect succeeded.

I'm reasonably sure that's what caused this failure: https://helix.dot.net/api/2019-06-17/jobs/e63fcc11-e9ad-476c-9798-47f2f6d3b152/workitems/System.Net.Http.Functional.Tests/console

@geoffkizer geoffkizer added this to the 5.0.0 milestone Aug 15, 2020
@ghost
Copy link

ghost commented Aug 15, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Aug 15, 2020
@karelz karelz added bug and removed untriaged New issue has not been triaged by the area owner labels Aug 15, 2020
@ManickaP
Copy link
Member

This has been already fixed in #39514
But returned back, probably via bad resolution of conflicts in merge, in #39524.

I can fix that.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants