Deallocation of SqlDataReader

Question

I have a contractor that wrote code like this all over the place when we execute sql queries:

sql.Append(string.Format("SELECT TableId FROM ps_SavedTables WHERE guid = '{0}'", guid));

        using (IDataReader reader = SqlHelper.GetDataReader(sql.ToString())) {
            if (reader.Read()) {
                result = reader.IsDBNull(0) ? string.Empty : reader[0].ToString();
            }
            //CDW added to close SqlDataReader
            reader.Close();
        }

The GetDataReader class was in an App_Code helper file and is declared like so:

 public static SqlDataReader GetDataReader(string sql) {
        return GetDataReader(sql, DefaultConnectionString);
    }

    public static SqlDataReader GetDataReader(string sql, int connectionTime) {
        return GetDataReader(sql, DefaultConnectionString, connectionTime);
    }

    /// <summary>
    /// Executes DB command ExecutedReader and returns the DataReader
    /// </summary>
    /// <param name="sql">SQL statement(s) to be used with DB command</param>
    /// <param name="connectionString">Database connection string</param>
    /// <returns>DataReader containing results of supplied SQL statement(s)</returns>
    public static SqlDataReader GetDataReader(string sql, string connectionString) {
        lock (_lock) {
            SqlConnection connection = null;
            try {
                connection = GetConnection(connectionString);
                //connection.Open();
                using (SqlCommand cmd = new SqlCommand(sql, connection)) {
                    WriteDebugInfo("GetDataReader", sql);
                    return cmd.ExecuteReader(CommandBehavior.CloseConnection);
                }
            }
            catch (Exception e) {
                if (connection != null)
                    connection.Dispose();
                throw new DataException(sql, connectionString, e);
            }

        }
    }

Then I run my code locally (it does a ton of SQL for statistics) and run the same type of thing in two different browsers, 1 browser always crashes here:

Server.ScriptTimeout = 300;
        string returnCode = string.Empty;
        using (SqlConnection connection = new SqlConnection(ConfigurationManager.ConnectionStrings["MainDll"].ToString())) {
            connection.Open();
            using (SqlCommand command = new SqlCommand(sql.ToString(), connection)) {
                command.CommandType = CommandType.Text;
                command.CommandTimeout = 300;
                returnCode = (string)command.ExecuteScalar();
                Dispose();
            }
            Dispose();
        }

The stack tace is this:

.NET Framework execution was aborted by escalation policy because of out of memory. 
System.InvalidOperationException: There is already an open DataReader associated with this Command which must be closed first.
System.InvalidOperationException: 
  at System.Data.SqlClient.SqlInternalConnectionSmi.ValidateConnectionForExecute(SqlCommand command)
  at System.Data.SqlClient.SqlConnection.ValidateConnectionForExecute(String method, SqlCommand command)
  at System.Data.SqlClient.SqlCommand.ValidateCommand(String method, Boolean async)
  at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method, DbAsyncResult result)
  at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
  at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior, String method)
  at System.Data.SqlClient.SqlCommand.ExecuteReader()
  at PowerStatRegression.share.JbLog.SendResultSet(String str, SqlConnection conn)
  at StoredProcedures.SavePowerStatErrors(ApplicationContext app, SqlString strUID, Int32 seqId, List`1 errors, Int32 errorType)
  at StoredProcedures.CreatePowerStatCorr(SqlString strUID, Int32 seqId, Int32 flagProgress)
  at StoredProcedures.SpCreatePowerStatCorr(SqlString strUID, Int32 seqId, Int32 flagProgress)

Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code. 

Exception Details: System.Data.SqlClient.SqlException: .NET Framework execution was aborted by escalation policy because of out of memory. 
System.InvalidOperationException: There is already an open DataReader associated with this Command which must be closed first.
System.InvalidOperationException: 
  at System.Data.SqlClient.SqlInternalConnectionSmi.ValidateConnectionForExecute(SqlCommand command)
  at System.Data.SqlClient.SqlConnection.ValidateConnectionForExecute(String method, SqlCommand command)
  at System.Data.SqlClient.SqlCommand.ValidateCommand(String method, Boolean async)
  at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method, DbAsyncResult result)
  at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
  at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior, String method)
  at System.Data.SqlClient.SqlCommand.ExecuteReader()
  at PowerStatRegression.share.JbLog.SendResultSet(String str, SqlConnection conn)
  at StoredProcedures.SavePowerStatErrors(ApplicationContext app, SqlString strUID, Int32 seqId, List`1 errors, Int32 errorType)
  at StoredProcedures.CreatePowerStatCorr(SqlString strUID, Int32 seqId, Int32 flagProgress)
  at StoredProcedures.SpCreatePowerStatCorr(SqlString strUID, Int32 seqId, Int32 flagProgress)


Source Error: 


Line 705:                    command.CommandType = CommandType.Text;
Line 706:                    command.CommandTimeout = 300;
Line 707:                    returnCode = (string)command.ExecuteScalar();
Line 708:                    //Dispose();
Line 709:                }


Source File: c:\PowerStats\datalab\powerstats\output.aspx.cs    Line: 707 

Stack Trace: 

    [SqlException (0x80131904): .NET Framework execution was aborted by escalation policy because of out of memory. 
System.InvalidOperationException: There is already an open DataReader associated with this Command which must be closed first.
System.InvalidOperationException: 
   at System.Data.SqlClient.SqlInternalConnectionSmi.ValidateConnectionForExecute(SqlCommand command)
   at System.Data.SqlClient.SqlConnection.ValidateConnectionForExecute(String method, SqlCommand command)
   at System.Data.SqlClient.SqlCommand.ValidateCommand(String method, Boolean async)
   at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method, DbAsyncResult result)
   at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
   at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior, String method)
   at System.Data.SqlClient.SqlCommand.ExecuteReader()
   at PowerStatRegression.share.JbLog.SendResultSet(String str, SqlConnection conn)
   at StoredProcedures.SavePowerStatErrors(ApplicationContext app, SqlString strUID, Int32 seqId, List`1 errors, Int32 errorType)
   at StoredProcedures.CreatePowerStatCorr(SqlString strUID, Int32 seqId, Int32 flagProgress)
   at StoredProcedures.SpCreatePowerStatCorr(SqlString strUID, Int32 seqId, Int32 flagProgress)
]
   System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection) +1950522
   System.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection) +4856715
   System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj) +194
   System.Data.SqlClient.TdsParser.Run(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj) +1121
   System.Data.SqlClient.SqlDataReader.ConsumeMetaData() +33
   System.Data.SqlClient.SqlDataReader.get_MetaData() +83
   System.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString) +293
   System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async) +954
   System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method, DbAsyncResult result) +162
   System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method) +32
   System.Data.SqlClient.SqlCommand.ExecuteScalar() +139
   Output.RunTable(String outputType, String _outputDataType) in c:\PowerStats\datalab\powerstats\output.aspx.cs:707
   Output.ProcessPage() in c:\PowerStats\datalab\powerstats\output.aspx.cs:652
   Output.Page_Load(Object sender, EventArgs e) in c:\PowerStats\datalab\powerstats\output.aspx.cs:644
   System.Web.Util.CalliHelper.EventArgFunctionCaller(IntPtr fp, Object o, Object t, EventArgs e) +14
   System.Web.Util.CalliEventHandlerDelegateProxy.Callback(Object sender, EventArgs e) +35
   System.Web.UI.Control.OnLoad(EventArgs e) +99
   System.Web.UI.Control.LoadRecursive() +50
   System.Web.UI.Page.ProcessRequestMain(Boolean includeStagesBeforeAsyncPoint, Boolean includeStagesAfterAsyncPoint) +627




--------------------------------------------------------------------------------
Version Information: Microsoft .NET Framework Version:2.0.50727.5456; ASP.NET Version:2.0.50727.5456 

Why isn't the SqlDataReader being deallocated correctly?

Answer 1

Does "lock" statement mean, that you're using this stuff from multiple threads?
Don't even try to share ADO .NET objects between threads - they are not thread safe... It seems like more than one thread creates reader instance the same time.

Answer 2

I think the lock is at the wrong level... The lock needs to be at the reader level. The open reader is being used after the lock is released.