SQL Server Closing SqlConnection in a ASP.NET and memory issue [duplicate]

dzhpxtsq  于 2023-10-15  发布在  .NET
关注(0)|答案(1)|浏览(147)

This question already has answers here:

Reason for Using IDisposable Interface (6 answers)
Closed 21 days ago.

The community reviewed whether to reopen this question 19 days ago and left it closed:

Original close reason(s) were not resolved

In the ASP.NET app I'm working with the data layer that has functions such as:

public DataSet GetFacilitiesInfo(string userName = null)
{
    DataSet Ds = new DataSet();
    string storedProcName = "FacilitySelect";

    SqlConnection sqlConn = new SqlConnection();
    SqlCommand sqlCommand;
            
    try
    {
        DBConnection cn = new DBConnection();
        sqlConn = cn.DBConnTIP(_connectionString);
        sqlCommand = new SqlCommand(storedProcName, sqlConn);
        sqlCommand.CommandType = CommandType.StoredProcedure;               

        SqlDataAdapter sqlDA = new SqlDataAdapter(sqlCommand);
        sqlDA.Fill(Ds);

        return Ds;
    }
    catch (Exception e)
    {
        throw e;
    }
    finally
    {
        sqlConn.Close();
        sqlConn.Dispose();
    }
}

Per Microsoft

The Dispose method is primarily implemented to release unmanaged resources.

I know in the above code we can use the using { ... } but behind the scenes, CIL translates it to exactly the same function calls.

My understanding is that SqlConnection (ADO.NET) is a managed object, so why do we dispose of it?

The suggested duplicate question has nothing to do with my question at all.

To improve this post I suggest putting the following as duplicate, at least it has some similarity:

Is SqlCommand.Dispose() required if associated SqlConnection will be disposed?

i7uq4tfw

i7uq4tfw1#

My understanding is that SqlConnection (ADO.NET) is a managed object, so why do we dispose of it?

The "managed" word here only refers to memory. So the memory used for the Sqlconnection object is managed and will be released appropriately, but the network, file, special shared memory, or named pipe resources used for completing the connection are not managed, and need to be disposed properly.

In fact, anything that implements IDisposable should either:

  1. Be wrapped in a using block
  2. Be wrapped in try / finally blocks
  3. Be used as a member of a type that itself implements IDisposable .

Always

(And the corollary is IDisposable should only be used when there's an unmanaged resource of some kind involved.)

As for the finally block, in this case, yes, the finally block is enough, and using is not strictly required.

However, the catch block here does absolutely nothing useful... worse, it destroys the call stack for any eventual exception. In other words, it's not only safe to just remove the catch block, but you're actively better off if you do.

Once that's done you can save even more code by also completely replacing both the try and finally blocks with a using block, and from there we can save another two lines, as well as a level of indentation, with the new using var construct.

Lastly, the sqlConn variable is over-written before ever using the new SqlConnection object allocated when it's first declared — the object is completely wasted. So we can clean that up, too.

Put it all together like this:

public DataSet GetFacilitiesInfo(string userName = null)
{
    DataSet Ds = new DataSet();
    DBConnection cn = new DBConnection();

    using var sqlConn = cn.DBConnTIP(_connectionString);
    using var sqlCommand = new SqlCommand("FacilitySelect", sqlConn);
    using var sqlDa = new SqlDataAdapter(sqlCommand);
        
    sqlDA.Fill(Ds);            
    return Ds;
}

This is safer (all the IDisposables are accounted for), has better error handling (call stack is preserved), uses noticeably less code (always a win), and with absolutely no loss of function.

For completeness, here's a version that also takes a few guesses to make use of the userName argument:

public DataSet GetFacilitiesInfo(string userName = null)
{
    DataSet Ds = new DataSet();
    DBConnection cn = new DBConnection();

    using var sqlConn = cn.DBConnTIP(_connectionString);
    using var sqlCommand = new SqlCommand("FacilitySelect", sqlConn);
    using var sqlDa = new SqlDataAdapter(sqlCommand);
    
    sqlCommand.CommandType = CommandType.StoredProcedure;  
    // I have to guess the type and length
    sqlCommand.Parameters.Add("@UserName", SqlDbType.NVarchar, 25).Value = userName;
    if (userName == null) sqlCommand.Parameters["@UserName"].Value = DBNull.Value;
        
    sqlDA.Fill(Ds);            
    return Ds;
}

That said, it is somewhat common to want actual meaningful error handling in this type of method, with a real catch block. And once you do that, you're back to where a finally block is just as good.

相关问题