Skip to content

Commit 9ed3525

Browse files
0xced0xced
authored
Do not close the inner reader when disposing wrapped data readers (#2100)
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() ```
1 parent 52160dc commit 9ed3525

File tree

2 files changed

+158
-2
lines changed

2 files changed

+158
-2
lines changed

‎Dapper/WrappedReader.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ protected override void Dispose(bool disposing)
129129
{
130130
if (disposing)
131131
{
132-
_reader.Close();
133132
_reader.Dispose();
134133
_reader = DisposedReader.Instance; // all future ops are no-ops
135134
_cmd?.Dispose();
@@ -245,7 +244,6 @@ protected override void Dispose(bool disposing)
245244
{
246245
if (disposing)
247246
{
248-
_reader.Close();
249247
_reader.Dispose();
250248
_reader = DisposedReader.Instance; // all future ops are no-ops
251249
}

‎tests/Dapper.Tests/WrappedReaderTests.cs

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
using System;
2+
using System.Collections;
3+
using System.Data;
4+
using System.Data.Common;
5+
using Xunit.Abstractions;
6+
7+
namespace Dapper.Tests;
8+
9+
public class WrappedReaderTests(ITestOutputHelper testOutputHelper)
10+
{
11+
[Fact]
12+
public void DbWrappedReader_Dispose_DoesNotThrow()
13+
{
14+
var reader = new DbWrappedReader(new DummyDbCommand(), new ThrowOnCloseDbDataReader(testOutputHelper));
15+
reader.Dispose();
16+
}
17+
18+
#if !NETFRAMEWORK
19+
[Fact]
20+
public async System.Threading.Tasks.Task DbWrappedReader_DisposeAsync_DoesNotThrow()
21+
{
22+
var reader = new DbWrappedReader(new DummyDbCommand(), new ThrowOnCloseDbDataReader(testOutputHelper));
23+
await reader.DisposeAsync();
24+
}
25+
#endif
26+
27+
[Fact]
28+
public void WrappedBasicReader_Dispose_DoesNotThrow()
29+
{
30+
var reader = new WrappedBasicReader(new ThrowOnCloseIDataReader());
31+
reader.Dispose();
32+
}
33+
34+
#if !NETFRAMEWORK
35+
[Fact]
36+
public async System.Threading.Tasks.Task WrappedBasicReader_DisposeAsync_DoesNotThrow()
37+
{
38+
var reader = new WrappedBasicReader(new ThrowOnCloseIDataReader());
39+
await reader.DisposeAsync();
40+
}
41+
#endif
42+
43+
private class DummyDbCommand : DbCommand
44+
{
45+
public override void Cancel() => throw new NotSupportedException();
46+
public override int ExecuteNonQuery() => throw new NotSupportedException();
47+
public override object ExecuteScalar() => throw new NotSupportedException();
48+
public override void Prepare() => throw new NotSupportedException();
49+
public override string CommandText { get; set; }
50+
public override int CommandTimeout { get; set; }
51+
public override CommandType CommandType { get; set; }
52+
public override UpdateRowSource UpdatedRowSource { get; set; }
53+
protected override DbConnection? DbConnection { get; set; }
54+
protected override DbParameterCollection DbParameterCollection => throw new NotSupportedException();
55+
protected override DbTransaction? DbTransaction { get; set; }
56+
public override bool DesignTimeVisible { get; set; }
57+
protected override DbParameter CreateDbParameter() => throw new NotSupportedException();
58+
protected override DbDataReader ExecuteDbDataReader(CommandBehavior behavior) => throw new NotSupportedException();
59+
}
60+
61+
private class DummyDbException(string message) : DbException(message);
62+
63+
private class ThrowOnCloseDbDataReader(ITestOutputHelper testOutputHelper) : DbDataReader
64+
{
65+
// 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
66+
protected override void Dispose(bool disposing)
67+
{
68+
try
69+
{
70+
if (disposing)
71+
{
72+
Close();
73+
}
74+
base.Dispose(disposing);
75+
}
76+
catch (DbException e)
77+
{
78+
testOutputHelper.WriteLine($"Ignored exception when disposing {e}");
79+
}
80+
}
81+
82+
public override void Close() => throw new DummyDbException("Exception during Close()");
83+
84+
public override bool GetBoolean(int ordinal) => throw new NotSupportedException();
85+
public override byte GetByte(int ordinal) => throw new NotSupportedException();
86+
public override long GetBytes(int ordinal, long dataOffset, byte[]? buffer, int bufferOffset, int length) => throw new NotSupportedException();
87+
public override char GetChar(int ordinal) => throw new NotSupportedException();
88+
public override long GetChars(int ordinal, long dataOffset, char[]? buffer, int bufferOffset, int length) => throw new NotSupportedException();
89+
public override string GetDataTypeName(int ordinal) => throw new NotSupportedException();
90+
public override DateTime GetDateTime(int ordinal) => throw new NotSupportedException();
91+
public override decimal GetDecimal(int ordinal) => throw new NotSupportedException();
92+
public override double GetDouble(int ordinal) => throw new NotSupportedException();
93+
public override Type GetFieldType(int ordinal) => throw new NotSupportedException();
94+
public override float GetFloat(int ordinal) => throw new NotSupportedException();
95+
public override Guid GetGuid(int ordinal) => throw new NotSupportedException();
96+
public override short GetInt16(int ordinal) => throw new NotSupportedException();
97+
public override int GetInt32(int ordinal) => throw new NotSupportedException();
98+
public override long GetInt64(int ordinal) => throw new NotSupportedException();
99+
public override string GetName(int ordinal) => throw new NotSupportedException();
100+
public override int GetOrdinal(string name) => throw new NotSupportedException();
101+
public override string GetString(int ordinal) => throw new NotSupportedException();
102+
public override object GetValue(int ordinal) => throw new NotSupportedException();
103+
public override int GetValues(object[] values) => throw new NotSupportedException();
104+
public override bool IsDBNull(int ordinal) => throw new NotSupportedException();
105+
public override int FieldCount => throw new NotSupportedException();
106+
public override object this[int ordinal] => throw new NotSupportedException();
107+
public override object this[string name] => throw new NotSupportedException();
108+
public override int RecordsAffected => throw new NotSupportedException();
109+
public override bool HasRows => throw new NotSupportedException();
110+
public override bool IsClosed => throw new NotSupportedException();
111+
public override bool NextResult() => throw new NotSupportedException();
112+
public override bool Read() => throw new NotSupportedException();
113+
public override int Depth => throw new NotSupportedException();
114+
public override IEnumerator GetEnumerator() => throw new NotSupportedException();
115+
}
116+
117+
private class ThrowOnCloseIDataReader : IDataReader
118+
{
119+
public void Dispose()
120+
{
121+
// Assume that IDataReader Dispose implementation does not throw
122+
}
123+
124+
public void Close() => throw new DummyDbException("Exception during Close()");
125+
126+
public bool GetBoolean(int i) => throw new NotSupportedException();
127+
public byte GetByte(int i) => throw new NotSupportedException();
128+
public long GetBytes(int i, long fieldOffset, byte[]? buffer, int bufferoffset, int length) => throw new NotSupportedException();
129+
public char GetChar(int i) => throw new NotSupportedException();
130+
public long GetChars(int i, long fieldoffset, char[]? buffer, int bufferoffset, int length) => throw new NotSupportedException();
131+
public IDataReader GetData(int i) => throw new NotSupportedException();
132+
public string GetDataTypeName(int i) => throw new NotSupportedException();
133+
public DateTime GetDateTime(int i) => throw new NotSupportedException();
134+
public decimal GetDecimal(int i) => throw new NotSupportedException();
135+
public double GetDouble(int i) => throw new NotSupportedException();
136+
public Type GetFieldType(int i) => throw new NotSupportedException();
137+
public float GetFloat(int i) => throw new NotSupportedException();
138+
public Guid GetGuid(int i) => throw new NotSupportedException();
139+
public short GetInt16(int i) => throw new NotSupportedException();
140+
public int GetInt32(int i) => throw new NotSupportedException();
141+
public long GetInt64(int i) => throw new NotSupportedException();
142+
public string GetName(int i) => throw new NotSupportedException();
143+
public int GetOrdinal(string name) => throw new NotSupportedException();
144+
public string GetString(int i) => throw new NotSupportedException();
145+
public object GetValue(int i) => throw new NotSupportedException();
146+
public int GetValues(object[] values) => throw new NotSupportedException();
147+
public bool IsDBNull(int i) => throw new NotSupportedException();
148+
public int FieldCount => throw new NotSupportedException();
149+
public object this[int i] => throw new NotSupportedException();
150+
public object this[string name] => throw new NotSupportedException();
151+
public DataTable? GetSchemaTable() => throw new NotSupportedException();
152+
public bool NextResult() => throw new NotSupportedException();
153+
public bool Read() => throw new NotSupportedException();
154+
public int Depth => throw new NotSupportedException();
155+
public bool IsClosed => throw new NotSupportedException();
156+
public int RecordsAffected => throw new NotSupportedException();
157+
}
158+
}

0 commit comments

Comments
 (0)