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

GetDbConnection() and connection pool management #7810

Closed
bragma opened this issue Mar 8, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@bragma
Copy link

commented Mar 8, 2017

Hi, I've seen many uses of GetDbConnection() wrapped in a using block. Since disposing the returned connection closes it, I am expecting this is required for some resource management in the connection pool but I've found no explicit explanation of what GetDbConnection does internally (ex: increment a reference counter) and if the "using" is really required in case DI is used for managing the (scoped) lifecycle of the DbContext.

Ex:
In an ASP NET Core app I interweave some use of EF and Dapper and I am in doubt on how to use GetDbConnection(). The pattern I've found on the net is:

using (var conn = _dbContext.Database.GetDbConnection())
{
	if (conn.State != ConnectionState.Open)
	{
		await conn.OpenAsync();
	}

	// Dapper query here
}

But this causes problems since after the "using" EF does not work anymore (the connection is closed) in the same scope (asp next controller). I was considering if the using can be removed and still not cause any connection leak:

var conn = _dbContext.Database.GetDbConnection();
if (conn.State != ConnectionState.Open)
{
	await conn.OpenAsync();
}

// Dapper query here

i.e., can the using be safely removed and still expect connection to be properly handled when the asp net controller method using the DbContext has completed?
Thanks!

@ajcvickers

This comment has been minimized.

Copy link
Member

commented Mar 8, 2017

@bragma If EF creates the DbConnection object, then EF will ensure it is disposed when the DbContext is disposed. On the other hand, if some other code creates the DbConnection object and passes it to EF, then it is the responsibility of the other code to also dispose the connection appropriately.

Similar rules apply to opening and closing the connection. If EF opens the connection, then EF will close the connection when it is done with it. If your code opens the connection, then your code should close the connection.

@ajcvickers

This comment has been minimized.

Copy link
Member

commented Mar 8, 2017

@bragma Yes, although it would be considered best practice to pair an explicit open and close.

@bragma

This comment has been minimized.

Copy link
Author

commented Mar 9, 2017

Sorry, I have mistakenly deleted my second question. As a reference, I am adding it back:

Given that DI will dispose DbContext and this in turn will dispose the EF created DbConnection in it, if the DbConnection is obtained via GetDbConnection() can we assume that even if it opened explicitly with conn.open() it will be closed automatically when DbConnection is disposed (so in this case when DbContext will be disposed)?

Answer by @ajcvickers is:

"Yes, although it would be considered best practice to pair an explicit open and close."

Thanks a lot!

@peteralbanese

This comment has been minimized.

Copy link

commented Apr 27, 2018

I was having the same issue as above and changed my code to the following. When the context.Database.GetDbConnection() is called in a Using Statement, it works the first time, but all subsequent calls to cn.Close on that Connection will fail. So I modified my code to NOT use the Using Statement and it worked. Hope this helps someone.

public static DataTable GetDataTable (this DbContext context, string sql, Dictionary<string, object> parameters)
{

        DataTable dt = new DataTable();
        DbConnection cn = context.Database.GetDbConnection();

        using (DbCommand cmd = cn.CreateCommand())
        {
            cmd.CommandText = sql;
            if (parameters != null)
            {
                foreach (var parameter in parameters)
                {
                    DbParameter dbParam = cmd.CreateParameter();
                    dbParam.ParameterName = parameter.Key;
                    dbParam.Value = parameter.Value;
                    cmd.Parameters.Add(dbParam);
                }
                if (cn.State.Equals(ConnectionState.Closed)) { cn.Open(); }
                using (DbDataReader reader = cmd.ExecuteReader())
                {
                    dt.Load(reader);
                }
            }
        }

        if (cn.State.Equals(ConnectionState.Open)) { cn.Close(); }

        return dt;
    }

cazangabriel added a commit to MountainWarehouse/EasyMWS that referenced this issue May 4, 2018

Removed potential cause for connection leak
aspnet/EntityFrameworkCore#6581
aspnet/EntityFrameworkCore#7810

whenever this GetDbConnection() would be called it should also be disposed of. therefore this way of setting the connection string is not valid.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.