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

ExecuteReaderAsync with CommandBehavior.CloseConnection does not close the connection #129

Closed
vankampenp opened this issue Jun 26, 2019 · 5 comments
Assignees
Labels
🐛 Bug! Something isn't right !
Milestone

Comments

@vankampenp
Copy link

Describe the bug

I am getting an exception on obtaining a connection. It looks as if ExecuteReaderAsync with CommandBehavior.CloseConnection does not close the connection.

If you are seeing an exception, include the full exceptions details (message and stack trace).
System.InvalidOperationException: Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.
at Microsoft.Data.Common.ADP.ExceptionWithStackTrace(Exception e)
--- End of stack trace from previous location where exception was thrown ---
at DTOWEB.Dal.SqlHelperAsync.ExecuteReaderAsync(String connectionString, CommandType commandType, String commandText, SqlParameter[] commandParameters)
at DTOWEB.Dal.TblTests.FindStateMainUserList()|System.InvalidOperationException: Timeout expired. The timeout period elapsed prior to obtaining a connection from the pool. This may have occurred because all pooled connections were in use and max pool size was reached.
at Microsoft.Data.Common.ADP.ExceptionWithStackTrace(Exception e)


To reproduce

public static async Task<List<StateUser>> FindStateMainUserList()
{
	const string qry = "some sql select qry"

	try
    {
        SqlParameter[] parametersArray = { };

        List<StateUser> listusers = new List<StateUser>();

        using (DbDataReader reader = await SqlHelperAsync.ExecuteReaderAsync(Config.DbConnectionString, CommandType.Text, qry, parametersArray))
        {
            while (reader.Read())
            {
                StateUser user = new StateUser
                {
                    .. fill user object parameters using the reader
                };
                listusers.Add(user);

            }
        }

        return listusers;
    }
    catch (Exception ex)
    {
        Logger.Error(ex);
        return null;
    }
}

 public static async Task<SqlDataReader > ExecuteReaderAsync(string connectionString, CommandType commandType, string commandText, params SqlParameter[] commandParameters)
    {
        //create & open an SqlbConnection
        SqlConnection cn = new SqlConnection(connectionString);
        await cn.OpenAsync();

        try
        {
            //call the private overload that takes an internally owned connection in place of the connection string
            return await ExecuteReaderAsync(cn, null, commandType, commandText, commandParameters, SqlConnectionOwnership.Internal);
        }
        catch (Exception ex)
        {
            //if we fail to return the SqlDataReader , we neeed to close the connection ourselves
            cn.Close();
         
            Logger.Error(commandText);
            Logger.Error(ex);
            return null;
        }
    } 
	
	 private static async Task<SqlDataReader> ExecuteReaderAsync(SqlConnection connection, SqlTransaction transaction, CommandType commandType, string commandText, SqlParameter[] commandParameters, SqlConnectionOwnership connectionOwnership)
    {
        //create a command and prepare it for execution
        SqlCommand cmd = new SqlCommand();
        await PrepareCommandAsync(cmd, connection, transaction, commandType, commandText, commandParameters);

        //create a reader
        SqlDataReader  dr;

        // call ExecuteReader with the appropriate CommandBehavior
        if (connectionOwnership == SqlConnectionOwnership.External)
        {
            dr = await cmd.ExecuteReaderAsync();
        }
        else
        {
            dr = await cmd.ExecuteReaderAsync(CommandBehavior.CloseConnection);
        }

        return dr;
    }
	
	 private static async Task PrepareCommandAsync(SqlCommand command, SqlConnection connection, SqlTransaction transaction, CommandType commandType, string commandText, SqlParameter[] commandParameters)
    {

        //if the provided connection is not open, we will open it
        if (connection.State != ConnectionState.Open)
        {
            await connection.OpenAsync();
        }

        //associate the connection with the command
        command.Connection = connection;

        //set the command text (stored procedure name or Sql statement)
        command.CommandText = commandText;


        //if we were provided a transaction, assign it.
        if (transaction != null)
        {
            command.Transaction = transaction;
        }

        //set the command type
        command.CommandType = commandType;

        //attach the command parameters if they are provided
        if (commandParameters != null)
        {
            AttachParameters(command, commandParameters);
        }

    }

This happens at many queries, so the exact sql statement was not relevant. But it was all the time at executing an ExecuteReaderAsync with CommandBehavior.CloseConnection.

The error happens fast, even after a few seconds (with repeated ExecuteReaderAsync)

Expected behavior

This occurred when changing System.Data.SqlClient (v4.6.1) to Microsoft.Data.SqlClient. v1.0.19128.1-Preview. There were many other changes as well.
After reverting back to System.Data.SqlClient the error has not occurred again.
Confirmed by moving again to Microsoft.Data.SqlClient and back to System.Data.SqlClient.

Further technical details

Microsoft.Data.SqlClient version: v1.0.19128.1-Preview (also tried v1.0.19123.2-Preview with same result)
System.Data.SqlClient v4.6.1 - no error
.NET target: (Core 2.2.5)
SQL Server version:SQL Server 2016
Operating system: Windows 2016,
IIS running out of proc
Deployment Release or Debug Any CPU TargetFramework netcoreapp2.2

Additional context
Add any other context about the problem here.

@vankampenp
Copy link
Author

CoreCommandBehavior.zip

Attached a VS 2019 project that reproduces the issue. In the HomeController, correct the connectionstring to point to a SQL database. Correct the sqlQuery to point to any table.

Start the project in project debugging mode. Click on System Reader, and watch the counter go up to 999 (1000 times the async read of 1 line in the table, or 1000 times opened a connection).

Now click on Microsoft Reader. This uses Microsoft.Data.SQL instead. Watch the counter going up to 99, where the pool is exhausted and an exception thrown. You can also notice the slow speed, which is probably from opening a new connection each time.

@David-Engel
Copy link
Contributor

@vankampenp Thanks for the submission and repro. We'll look into the issue as soon as we can.

CC: @cheenamalhotra

@vankampenp
Copy link
Author

@vankampenp Thanks for the submission and repro. We'll look into the issue as soon as we can.

CC: @cheenamalhotra

@David-Engel I am not in a hurry, I can still use the System.Data.SqlClient until it is fixed.
I forgot to mention, but I first made a repo as a console app, but that codebase did not have the CloseConnection enum at all:

Error CS0234 The type or namespace name 'CloseConnection' does not exist in the namespace 'CommandBehavior' (are you missing an assembly reference?)
This sounds like a different issue, but it might be related.

@cheenamalhotra cheenamalhotra added this to the 1.0.0 milestone Jul 2, 2019
@cheenamalhotra cheenamalhotra added the 🐛 Bug! Something isn't right ! label Jul 2, 2019
@David-Engel David-Engel self-assigned this Jul 31, 2019
@David-Engel David-Engel moved this from To do to Review in progress in SqlClient v1.0.0 Jul 31, 2019
@cheenamalhotra cheenamalhotra moved this from Review in progress to Reviewer approved in SqlClient v1.0.0 Aug 2, 2019
@cheenamalhotra cheenamalhotra moved this from Reviewer approved to Done in SqlClient v1.0.0 Aug 2, 2019
@cheenamalhotra
Copy link
Member

Hi @vankampenp

We were able to fix the problem and verify the change with the provided repro, the next preview release is likely to fix this issue.

@cheenamalhotra
Copy link
Member

Preview release v1.0.19221.1 has been published containing fix. Closing issue.

JRahnama pushed a commit to JRahnama/SqlClient that referenced this issue Sep 5, 2019
Fix for ExecuteReaderAsync
[GitHub issue 129](dotnet#129): ExecuteReaderAsync with CommandBehavior.CloseConnection does not close the connection

I found that this was an oversight from my backport of 33660. 33660 had changes that overlapped with the AE changes and I missed this in the intersection.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug! Something isn't right !
Projects
No open projects
Development

No branches or pull requests

3 participants