Skip to content

Commit

Permalink
Do not close the inner reader when disposing wrapped data readers (#2100
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 authored Jul 3, 2024
1 parent 52160dc commit 9ed3525
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 9ed3525

Please sign in to comment.