Skip to content

Conversation

@sanych-sun
Copy link
Member

No description provided.

@sanych-sun sanych-sun requested a review from a team as a code owner November 26, 2024 01:18
@sanych-sun sanych-sun requested review from JamesKovacs and removed request for a team November 26, 2024 01:18
@sanych-sun sanych-sun requested review from BorisDog and removed request for JamesKovacs December 12, 2024 19:12
throw new TimeoutException(message);
}
if (state == 4) { throw new OperationCanceledException(); }
WaitHandle.WaitAny(new[] { connectOperation.AsyncWaitHandle, cancellationToken.WaitHandle }, _settings.ConnectTimeout);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: [connectOperation.AsyncWaitHandle, cancellationToken.WaitHandle]

if (endPoint is DnsEndPoint dnsEndPoint)
{
// mono doesn't support DnsEndPoint in its BeginConnect method.
connectTask = Task.Factory.FromAsync(socket.BeginConnect(dnsEndPoint.Host, dnsEndPoint.Port, null, null), socket.EndConnect);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check if ConnectAsync can replace BeginConnect (SocketTaskExtensions)

@sanych-sun sanych-sun requested a review from BorisDog December 19, 2024 20:12

if (!connectOperation.IsCompleted)
{
socket.Dispose();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to hide Dispose exceptions here, and probably in all cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

{
// a timeout or operation cancelled exception will be thrown instead
}
socket.Dispose();
Copy link
Contributor

Choose a reason for hiding this comment

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

Ignore exceptions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@BorisDog BorisDog left a comment

Choose a reason for hiding this comment

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

LGTM!

@sanych-sun sanych-sun merged commit 2349b02 into mongodb:main Dec 20, 2024
22 of 25 checks passed
@sanych-sun sanych-sun deleted the csharp3840 branch December 20, 2024 18:04
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