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

Change connection task error handling. #873

Merged
merged 8 commits into from
Mar 7, 2024
Merged

Conversation

scottf
Copy link
Collaborator

@scottf scottf commented Mar 6, 2024

Analysis

When a TLS connection fails, in this case the server being connected to is not even on-line, the next connection attempt sometimes freezes. It was intermittent, but reproducible. See code at end.

With some debugging, I could see where it was freezing, but it looked like it was inside the .net library code. What I noticed is that the connect code seemed overly complicated to me, and there was this GC.KeepAlive(t.Exception);. At the end of the day, we don't care why it failed, we just want to move on. So I simplified the code.

There was a corresponding TLS Handshake error in the server log.

[114772] 2024/03/06 15:16:37.763950 [DBG] [::1]:64363 - cid:20 - Starting TLS client connection handshake
[114772] 2024/03/06 15:16:37.763950 [ERR] [::1]:64363 - cid:20 - TLS handshake error: read tcp [::1]:4222->[::1]:64363: wsarecv: An established connection was aborted by the software in your host machine.
[114772] 2024/03/06 15:16:37.764607 [DBG] [::1]:64363 - cid:20 - Client connection closed: TLS Handshake Failure

Since this change, I cannot reproduce the problem and the error is not appearing in the server log.

Code To Reproduce

static void Main(string[] args)
{
    Options opts = ConnectionFactory.GetDefaultOptions();
    opts.Servers = new [] {"tls://localhost:9222", "tls://localhost:4222"};
    opts.NoRandomize = true;
    opts.TlsFirst = true;
    opts.Secure = true;
    opts.TLSRemoteCertificationValidationCallback = (sndr, cert, chain, errs) => true;

    int round = 0;
    while (true)
    {
        Console.WriteLine($"Start Round {++round}");
        try
        {
            using (IConnection c = new ConnectionFactory().CreateConnection(opts, true))
            {
                Console.WriteLine("ServerInfo:" + c.ServerInfo);
            }
        }
        catch (Exception e)
        {
            Console.WriteLine(e.Message);
        }
        Console.WriteLine();
        Thread.Sleep(100);
    }
}

Copy link
Contributor

@mtmk mtmk left a comment

Choose a reason for hiding this comment

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

LGTM

@sixlettervariables
Copy link
Collaborator

sixlettervariables commented Mar 6, 2024

That code was to mark the exception as observed. Without reading task.Exception you'll end up with an unhandled exception. We likely don't want that.

Edit: I see where the move to Wait will raise this as an aggregate exception instead.

@scottf scottf merged commit c4dd08b into main Mar 7, 2024
1 check passed
@scottf scottf deleted the connection-task-error-handling branch March 7, 2024 00:03
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.

3 participants