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

Azure SQL DB connection hangs with zero-connection timeout #58

Closed
MaximLipnin opened this issue Apr 4, 2019 · 9 comments
Closed

Azure SQL DB connection hangs with zero-connection timeout #58

MaximLipnin opened this issue Apr 4, 2019 · 9 comments
Milestone

Comments

@MaximLipnin
Copy link

From mono/mono#13065

There is an issue when a connection to Azure SQL DB hangs on macOS if connection string contains Connection Timeout=0.

My repro sample is:

using System;
using System.Data.SqlClient;

namespace MyTest
{
    public class Repro
    {
        public static void Main()
        {
            OpenSqlConnection();
        }

        private static void OpenSqlConnection()
        {
            string connectionString = GetConnectionString();
            using (SqlConnection connection = new SqlConnection())
            {
                connection.ConnectionString = connectionString;
                connection.Open();
                Console.WriteLine("State: {0}", connection.State);
                Console.WriteLine("ConnectionString: {0}",  connection.ConnectionString);
            }
        }

        static private string GetConnectionString()
        {
            return "<some connection string stuff>;Connection Timeout=0;";
        }
    }
}

It's reproducible with Mono version which uses System.Data.SqlClient namespace from CoreFX. But there is no repro with Net Core 2.2.

It looks like it makes attempts to login and fails with System.ObjectDisposedException exception which is not the reason correctly to break login loop.

It occurs in non-parallel conditional branch of SNITCPHandle ctor:
https://github.com/dotnet/corefx/blob/master/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNITcpHandle.cs#L145

Further in case of zero timeout Cancel callback might be invoked (and dispose socket) before Socket.Connect:
https://github.com/dotnet/corefx/blob/master/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNITcpHandle.cs#L200-L216

Possible fix is to pass infinite timeout to Connect method https://github.com/dotnet/corefx/blob/master/src/System.Data.SqlClient/src/System/Data/SqlClient/SNI/SNITcpHandle.cs#L145:

_socket = Connect(serverName, port, isInfiniteTimeOut ? TimeSpan.FromMilliseconds(Int32.MaxValue) : ts);

Does it make sense for you?

/cc @afsanehr @marek-safar

@Wraith2
Copy link
Contributor

Wraith2 commented Apr 5, 2019

I was just looking at this code for other reasons. I understand that it's waiting indefinitely because thats what you asked it to do but I assume that it shouldn't be so what is the expected behaviour in your test case, does the connection succeed or get rejected? or does it not matter?

Out of interest can you try the same with async?

@MaximLipnin
Copy link
Author

I suppose that any standard result is expected behavior. In my case I was able to connect to Azure SQL DB successfully.

@roji
Copy link
Member

roji commented Apr 5, 2019

Out of interest can you try the same with async?

Am interested in hearing the result of that - as a general rule timeouts seem to be supported only when using sync I/O (see the docs on SqlCommand.CommandTimeout and on Socket.ReceiveTimeout).

@MaximLipnin
Copy link
Author

After changing Open to OpenAsync I still have the issue with hanging connection.

// ....
    //connection.Open();
    var connectionTask = connection.OpenAsync();
    Task.WaitAll(connectionTask);
// ...

@divega divega transferred this issue from dotnet/corefx May 16, 2019
@divega
Copy link

divega commented May 16, 2019

As recently announced in the .NET Blog, focus on new SqlClient features an improvements is moving to the new Microsoft.Data.SqlClient package. For this reason, we are moving this issue to the new repo at https://github.com/dotnet/SqlClient. We will still use https://github.com/dotnet/corefx to track issues on other providers like System.Data.Odbc and System.Data.OleDB, and general ADO.NET and .NET data access issues.

@David-Engel David-Engel added this to the 1.1.0 milestone May 20, 2019
@thiagobucca
Copy link

thiagobucca commented May 30, 2019

Just to be clear, that means i'll have to move my app from System.Data.SqlClient to Microsoft.Data.SqlClient to get this fix? Or there'll be a next mono release that'll stop using System.Data namespace? Since that is a bug (no new feature and neither an improvement), the fix shouldn't go to corefx?

@David-Engel
Copy link
Contributor

@thiagobucca Not necessarily. Depending on how the bug is prioritized, a fix in Microsoft.Data.SqlClient may be ported back to System.Data.SqlClient in CoreFx.

That said, looking at the logic you pointed to in SNITcpHandle, it makes me wonder why this doesn't manifest in .NET Core 2.2. I would think it would repro on Linux or mac there, too.

Priority-wise, this doesn't seem critical (simple workaround) so I dropped it into the 1.1.0 milestone for investigation after 1.0.

@thiagobucca
Copy link

@David-Engel Thank you for clarifying.

@karinazhou
Copy link
Member

Close this one since the fix will be available in the next release.

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

No branches or pull requests

8 participants