Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Do not close the inner reader when disposing wrapped data readers#2100

Merged
mgravell merged 1 commit intoDapperLib:mainDapperLib/Dapper:mainfrom
0xced:DisposeNoThrow0xced/Dapper:DisposeNoThrowCopy head branch name to clipboard
Jul 3, 2024
Merged

Do not close the inner reader when disposing wrapped data readers#2100
mgravell merged 1 commit intoDapperLib:mainDapperLib/Dapper:mainfrom
0xced:DisposeNoThrow0xced/Dapper:DisposeNoThrowCopy head branch name to clipboard

Conversation

@0xced
Copy link
Contributor

@0xced 0xced commented Jul 3, 2024

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()

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()
```
@mgravell
Copy link
Member

mgravell commented Jul 3, 2024

Seems fair; we can let the inner reader worry about what Dispose() means, since we're propagating Dispose(), not Close() + Dispose().

@mgravell
Copy link
Member

mgravell commented Jul 3, 2024

overriding appveyor, although the pgsql problem might prevent "main" deploy - that's not your problem, though; unrelated to this PR

@mgravell mgravell merged commit 9ed3525 into DapperLib:main Jul 3, 2024
@0xced 0xced deleted the DisposeNoThrow branch July 3, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.