Skip to content

Commit

Permalink
Do not close the inner reader when disposing wrapped data readers
Browse files Browse the repository at this point in the history 
When disposing wrapped readers, only call Dispose() on the inner reader and let it catch exceptions if appropriate.

Note: this actually happened with `Microsoft.Data.SqlClient`.

```
Microsoft.Data.SqlClient.SqlException (0x80131904): Execution Timeout Expired.  The timeout period elapsed prior to completion of the operation or the server is not responding.
 ---> System.ComponentModel.Win32Exception (258): The wait operation timed out.
   at void Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, bool breakConnection, Action<Action> wrapCloseInAction)
   at void Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, bool callerHasConnectionLock, bool asyncClose)
   at bool Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, out bool dataReady)
   at bool Microsoft.Data.SqlClient.SqlDataReader.TryCloseInternal(bool closeReader)
   at void Microsoft.Data.SqlClient.SqlDataReader.Close()
   at void Dapper.DbWrappedReader.Dispose(bool disposing) in /_/Dapper/WrappedReader.cs:line 149
   at ValueTask System.Data.Common.DbDataReader.DisposeAsync()
```
  • Loading branch information
@0xced
0xced committed Jul 3, 2024
1 parent 52160dc commit 81f4ba9
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 2 deletions.
2 changes: 0 additions & 2 deletions Dapper/WrappedReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ protected override void Dispose(bool disposing)
{
if (disposing)
{
_reader.Close();
_reader.Dispose();
_reader = DisposedReader.Instance; // all future ops are no-ops
_cmd?.Dispose();
Expand Down Expand Up @@ -245,7 +244,6 @@ protected override void Dispose(bool disposing)
{
if (disposing)
{
_reader.Close();
_reader.Dispose();
_reader = DisposedReader.Instance; // all future ops are no-ops
}
Expand Down
158 changes: 158 additions & 0 deletions tests/Dapper.Tests/WrappedReaderTests.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
using System;
using System.Collections;
using System.Data;
using System.Data.Common;
using Xunit.Abstractions;

namespace Dapper.Tests;

public class WrappedReaderTests(ITestOutputHelper testOutputHelper)
{
[Fact]
public void DbWrappedReader_Dispose_DoesNotThrow()
{
var reader = new DbWrappedReader(new DummyDbCommand(), new ThrowOnCloseDbDataReader(testOutputHelper));
reader.Dispose();
}

#if !NETFRAMEWORK
[Fact]
public async System.Threading.Tasks.Task DbWrappedReader_DisposeAsync_DoesNotThrow()
{
var reader = new DbWrappedReader(new DummyDbCommand(), new ThrowOnCloseDbDataReader(testOutputHelper));
await reader.DisposeAsync();
}
#endif

[Fact]
public void WrappedBasicReader_Dispose_DoesNotThrow()
{
var reader = new WrappedBasicReader(new ThrowOnCloseIDataReader());
reader.Dispose();
}

#if !NETFRAMEWORK
[Fact]
public async System.Threading.Tasks.Task WrappedBasicReader_DisposeAsync_DoesNotThrow()
{
var reader = new WrappedBasicReader(new ThrowOnCloseIDataReader());
await reader.DisposeAsync();
}
#endif

private class DummyDbCommand : DbCommand
{
public override void Cancel() => throw new NotSupportedException();
public override int ExecuteNonQuery() => throw new NotSupportedException();
public override object ExecuteScalar() => throw new NotSupportedException();
public override void Prepare() => throw new NotSupportedException();
public override string CommandText { get; set; }
public override int CommandTimeout { get; set; }
public override CommandType CommandType { get; set; }
public override UpdateRowSource UpdatedRowSource { get; set; }
protected override DbConnection? DbConnection { get; set; }
protected override DbParameterCollection DbParameterCollection => throw new NotSupportedException();
protected override DbTransaction? DbTransaction { get; set; }
public override bool DesignTimeVisible { get; set; }
protected override DbParameter CreateDbParameter() => throw new NotSupportedException();
protected override DbDataReader ExecuteDbDataReader(CommandBehavior behavior) => throw new NotSupportedException();
}

private class DummyDbException(string message) : DbException(message);

private class ThrowOnCloseDbDataReader(ITestOutputHelper testOutputHelper) : DbDataReader
{
// This is basically what SqlClient does, see https://github.com/dotnet/SqlClient/blob/v5.2.1/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs#L835-L849
protected override void Dispose(bool disposing)
{
try
{
if (disposing)
{
Close();
}
base.Dispose(disposing);
}
catch (DbException e)
{
testOutputHelper.WriteLine($"Ignored exception when disposing {e}");
}
}

public override void Close() => throw new DummyDbException("Exception during Close()");

public override bool GetBoolean(int ordinal) => throw new NotSupportedException();
public override byte GetByte(int ordinal) => throw new NotSupportedException();
public override long GetBytes(int ordinal, long dataOffset, byte[]? buffer, int bufferOffset, int length) => throw new NotSupportedException();
public override char GetChar(int ordinal) => throw new NotSupportedException();
public override long GetChars(int ordinal, long dataOffset, char[]? buffer, int bufferOffset, int length) => throw new NotSupportedException();
public override string GetDataTypeName(int ordinal) => throw new NotSupportedException();
public override DateTime GetDateTime(int ordinal) => throw new NotSupportedException();
public override decimal GetDecimal(int ordinal) => throw new NotSupportedException();
public override double GetDouble(int ordinal) => throw new NotSupportedException();
public override Type GetFieldType(int ordinal) => throw new NotSupportedException();
public override float GetFloat(int ordinal) => throw new NotSupportedException();
public override Guid GetGuid(int ordinal) => throw new NotSupportedException();
public override short GetInt16(int ordinal) => throw new NotSupportedException();
public override int GetInt32(int ordinal) => throw new NotSupportedException();
public override long GetInt64(int ordinal) => throw new NotSupportedException();
public override string GetName(int ordinal) => throw new NotSupportedException();
public override int GetOrdinal(string name) => throw new NotSupportedException();
public override string GetString(int ordinal) => throw new NotSupportedException();
public override object GetValue(int ordinal) => throw new NotSupportedException();
public override int GetValues(object[] values) => throw new NotSupportedException();
public override bool IsDBNull(int ordinal) => throw new NotSupportedException();
public override int FieldCount => throw new NotSupportedException();
public override object this[int ordinal] => throw new NotSupportedException();
public override object this[string name] => throw new NotSupportedException();
public override int RecordsAffected => throw new NotSupportedException();
public override bool HasRows => throw new NotSupportedException();
public override bool IsClosed => throw new NotSupportedException();
public override bool NextResult() => throw new NotSupportedException();
public override bool Read() => throw new NotSupportedException();
public override int Depth => throw new NotSupportedException();
public override IEnumerator GetEnumerator() => throw new NotSupportedException();
}

private class ThrowOnCloseIDataReader : IDataReader
{
public void Dispose()
{
// Assume that IDataReader Dispose implementation does not throw
}

public void Close() => throw new DummyDbException("Exception during Close()");

public bool GetBoolean(int i) => throw new NotSupportedException();
public byte GetByte(int i) => throw new NotSupportedException();
public long GetBytes(int i, long fieldOffset, byte[]? buffer, int bufferoffset, int length) => throw new NotSupportedException();
public char GetChar(int i) => throw new NotSupportedException();
public long GetChars(int i, long fieldoffset, char[]? buffer, int bufferoffset, int length) => throw new NotSupportedException();
public IDataReader GetData(int i) => throw new NotSupportedException();
public string GetDataTypeName(int i) => throw new NotSupportedException();
public DateTime GetDateTime(int i) => throw new NotSupportedException();
public decimal GetDecimal(int i) => throw new NotSupportedException();
public double GetDouble(int i) => throw new NotSupportedException();
public Type GetFieldType(int i) => throw new NotSupportedException();
public float GetFloat(int i) => throw new NotSupportedException();
public Guid GetGuid(int i) => throw new NotSupportedException();
public short GetInt16(int i) => throw new NotSupportedException();
public int GetInt32(int i) => throw new NotSupportedException();
public long GetInt64(int i) => throw new NotSupportedException();
public string GetName(int i) => throw new NotSupportedException();
public int GetOrdinal(string name) => throw new NotSupportedException();
public string GetString(int i) => throw new NotSupportedException();
public object GetValue(int i) => throw new NotSupportedException();
public int GetValues(object[] values) => throw new NotSupportedException();
public bool IsDBNull(int i) => throw new NotSupportedException();
public int FieldCount => throw new NotSupportedException();
public object this[int i] => throw new NotSupportedException();
public object this[string name] => throw new NotSupportedException();
public DataTable? GetSchemaTable() => throw new NotSupportedException();
public bool NextResult() => throw new NotSupportedException();
public bool Read() => throw new NotSupportedException();
public int Depth => throw new NotSupportedException();
public bool IsClosed => throw new NotSupportedException();
public int RecordsAffected => throw new NotSupportedException();
}
}

0 comments on commit 81f4ba9

Please sign in to comment.