Failed SqlConnection with Pooling drags down entire .NET Core MVC App

[h3smith]

When Pooling is enabled on a SqlConnection, and the connection fails (bad password, bad username, database doesn't exist) it will crash the entire .NET Core MVC app running.

For instance, the connection string: Password=mybadpassword;User Id=api_web_admn;Data Source=127.0.0.1,1433;Initial Catalog=MyDatabase;Integrated Security=False;Min Pool Size=1;Max Pool Size=200;Pooling=true;

produces:

Unhandled Exception: System.Data.SqlClient.SqlException: Login failed for user 'api_web_admn'.
   at System.Data.SqlClient.SqlInternalConnectionTds..ctor(DbConnectionPoolIdentity identity, SqlConnectionString connectionOptions, Object providerInfo, Boolean redirectedUserInstance, SqlConnectionString userConnectionOptions, SessionData reconnectSessionData, Boolean applyTransientFaultHandling)
   at System.Data.SqlClient.SqlConnectionFactory.CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, Object poolGroupProviderInfo, DbConnectionPool pool, DbConnection owningConnection, DbConnectionOptions userOptions)
   at System.Data.ProviderBase.DbConnectionFactory.CreatePooledConnection(DbConnectionPool pool, DbConnection owningObject, DbConnectionOptions options, DbConnectionPoolKey poolKey, DbConnectionOptions userOptions)
   at System.Data.ProviderBase.DbConnectionPool.CreateObject(DbConnection owningObject, DbConnectionOptions userOptions, DbConnectionInternal oldConnection)
   at System.Data.ProviderBase.DbConnectionPool.PoolCreateRequest(Object state)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()

and causes the .NET Core runtime to exit the thread.

Removing Pooling, or Pooling=false, it simply throws an exception and you can handle it.

As it stands now, you can't mitigate and handle a failed connection in .NET Core MVC.

[saurabh500]

@h3smith Can you provide details about the environment? What is the OS and .Net core version that you are using? If you have a sample app we could try out, that would be helpful.

[h3smith]

@saurabh500 I've tested this on Windows 2012 R2 (SQL Server 2012), Docker on OS X (Both SQL Server 2012 and SQL Server on Docker), AWS EC2 (Amazon RDS SQL Server). So a variety of environments. Doesn't seem to be tied to a single configuration.

[benaadams]

PoolCreateRequest has finally blocks so its expecting exceptions; but it does not catch any exceptions at the outer level so it throws an exception at the top level in the Threadpool which has no where to go so crashes by design as it has no where else to bubble up to.

The function should not let the exception escape (with catch) and either do something with it or throw it away.

[benaadams]

Same with PoolCreateRequest in System.Data.Odbc

[saurabh500]

@benaadams

The function should not let the exception escape (with catch) and either do something with it or throw it away.

The exceptions from SqlClient should be surfaced to the user by throwing it as it is happening right now. The exceptions are expected and would need to be surfaced. Even if a catch is added to the PoolCreateRequest, the exception cannot be thrown away. PoolCreateRequest cannot do anything with the exception anyhow.

It's up to the user to rectify the cause of the exception which could be bad credentials, bad network etc.

[benaadams]

Yes but QueuePoolCreateRequest sticks it in the threadpool so its at the top of the stack and has no caller to give the exception to; so it goes to the Process and the process crashes.

Same as if you threw an exception in the Main function and didn't catch it. You'll have to route it back in some way.

[saurabh500]

@benaadams Thanks for pointing out the code.

The Connection Pool for System.Data (used by ODBC and SqlClient) are relying on the ThreadPool handling the exception and returning the thread to the pool. Routing the exception some other way would be a breaking change with respect to how the exceptions are surfaced from the ConnectionPool. The alternate routing is definitely needed for MVC apps where the exception on thread pool may cause the application to crash, however the applications which expect the exceptions to be thrown, will break, especially when comes to interoperability of the app code between .Net Core and Framework.

I am working on a Repro and reading through the issues that you have linked to understand what decisions were taken around similar problems.

[benaadams]

At a guess for the PoolCreateRequest you want to wrap it in another function which catches the exception and sets _resError = e; and _errorOccurred = true;; but its probably more complicated than that.

e.g.

private void PoolCreateRequest(object state)
{
    try
    {
        // ...
    }
    catch (Exception ex)
    {
        _resError = ex;
        _errorOccurred = true;
    }
}

Whether it normally gets swallowed on the Framework as a AppDomain.UnhandledException or pops up randomly on a SynchronisationContext.Post probably isn't a helpful behaviour (ASP.NET Core has no SynchronisationContext catching the exceptions).

You may want to check Thread.CurrentThread.SynchronizationContext == null in the .ctor and if so change _poolCreateRequest = new WaitCallback(PoolCreateRequest); to use a wrapped version;

e.g.

private void WrappedPoolCreateRequest(object state)
{
    try
    {
        PoolCreateRequest(state);
    }
    catch (Exception ex)
    {
        _resError = ex;
        _errorOccurred = true;
    }
}

and in .ctor

if (Thread.CurrentThread.SynchronizationContext == null)
{
    _poolCreateRequest = new WaitCallback(WrappedPoolCreateRequest);
}
else
{
    _poolCreateRequest = new WaitCallback(PoolCreateRequest);
}

[geleems]

I was trying to repro the issue with this simple ASP.NET Core code.

<ItemGroup>
    <PackageReference Include="Microsoft.ApplicationInsights.AspNetCore" Version="2.0.0" />
    <PackageReference Include="Microsoft.AspNetCore" Version="1.1.1" />
    <PackageReference Include="Microsoft.AspNetCore.Mvc" Version="1.1.2" />
    <PackageReference Include="Microsoft.AspNetCore.StaticFiles" Version="1.1.1" />
    <PackageReference Include="Microsoft.Extensions.Logging.Debug" Version="1.1.1" />
    <PackageReference Include="Microsoft.VisualStudio.Web.BrowserLink" Version="1.1.0" />
    <PackageReference Include="System.Data.SqlClient" Version="4.3.0" />
    <PackageReference Include="System.Data.Common" Version="4.3.0" />
    <PackageReference Include="System.Runtime" Version="4.3.0" />
</ItemGroup>

Controller method:

public string Index()
{
    string result = "Success!!";
    string connString = "Server=tcp:<hostname>,1433;User ID=<id>;Password=<wrong_password>;Connect Timeout=600;pooling=true";
    using (SqlConnection conn = new SqlConnection(new SqlConnectionStringBuilder(connString).ConnectionString))
    {
        try
        {
            conn.Open();
        }
        catch(Exception e)
        {
            result = e.Message;
        }
    }

    return result;
}

The web app does not crash with wrong password, and browser simply shows this message on screen.

Login failed for user 'testuser'.

It seems it is already fixed.

[paulius-m]

Hello,
this code should reproduce bug:

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.All" Version="2.0.3" />
  </ItemGroup>

Controller method:

        public async Task<string> Index()
        {
            string result = "Success!!";
            string connString = "data source=**,35352;initial catalog=**;persist security info=False;Min Pool Size=1;user id=**; password=**;";
            using (SqlConnection conn = new SqlConnection(new SqlConnectionStringBuilder(connString).ConnectionString))
            {
                try
                {
                    await conn.OpenAsync();
                }
                catch (Exception e)
                {
                    result = e.Message;
                }
            }

            return result;
        }

[jvitkauskas]

@saurabh500 We are having this issue in production so it is critical for us. This effectively means we cannot use connection pooling in Core.

[saurabh500]

I think the issue is specific to the use case of warming up the connection pool, where the Min Pool Size is set to a non-zero value, and the connections are being populated in the connection pool using the Min Pool Size.
I have been able to repro the bug in cases where the Min Pool Size was set to a non-zero value (n) and there weren't n connections available in the pool.

[saurabh500]

In case the connection being opened in the

try { await sqlConnection.OpenAsync() } catch (Exception e) { } 

fails, the exception can be handled. However if the connection error occurs when the pool spins up threads to bring up the connection count to Min Pool Size, they lead to an application crash.

[EndarValuk]

Suppose still exists as a regression at 2.1.300-preview2-008533.
When connection string has something like this

var builder = new SqlConnectionStringBuilder
{
..omitted
MinPoolSize = 1,
MaxPoolSize = 10,
..omitted
};

And for an instance, no conection to server, whole app is going down with uncatchable exception

System.Data.SqlClient.SqlException: A network-related or instance-specific error occurred while establishing a connection to SQL Server. The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server is configured to allow remote connections. (provider: TCP Provider, error: 40 - Could not open a connection to SQL Server)
at System.Data.SqlClient.SqlInternalConnectionTds..ctor(DbConnectionPoolIdentity identity, SqlConnectionString connectionOptions, Object providerInfo, Boolean redirectedUserInstance, SqlConnectionString userConnectionOptions, SessionData reconnectSessionData, Boolean applyTransientFaultHandling)

OS: Ubuntu 18.04
Kernel: Linux 4.15.0-20-generic 21-Ubuntu SMP Tue Apr 24 06:16:15 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

[saurabh500]

@EndarValuk What version of SqlClient are you using?
The fix is available in 4.5.0-rc1 https://www.nuget.org/packages/System.Data.SqlClient/4.5.0-rc1

[EndarValuk]

@saurabh500 your truth. I was using SqlClient 4.4.3.
Updated to 4.5.0-rc1 and exception is gone. Thought fix was already in stable release(not found detailed release notes). Thank you.