Clear parameters when tearing down a command - #199
Merged
Conversation
Vanilla Dapper's finally blocks all do cmd.Parameters.Clear() before the command is disposed, and that is load-bearing rather than tidy: a parameter object the caller supplied - an ICustomQueryParameter's DbParameter, or one added to DynamicParameters directly - stays owned by the dead command's collection otherwise, and the next use throws 'The SqlParameter is already contained by another SqlParameterCollection'. UnifiedCommand.Cleanup now does the same; recycled commands keep their parameters, as before, since recycling does not pass through Cleanup.
UnifiedCommand.Cleanup was the wrong (or at least insufficient) place: the query and execute pipelines dispose the command via SyncCommandState / AsyncCommandState, which never pass through Cleanup. Recycled commands are nulled out of the state before Dispose runs, so their parameters are still kept for in-place update, as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Vanilla Dapper's finally blocks all do
cmd.Parameters.Clear()before the command is disposed (the "Add-tastic" comments), and it's load-bearing rather than tidy: a parameter object the caller supplied — anICustomQueryParameter'sDbParameter, or one added toDynamicParametersdirectly — otherwise stays owned by the dead command's collection, and the next use throws "The SqlParameter is already contained by another SqlParameterCollection". That's exactly what the Dapper test suite'sTestCustomParameterReusecatches once #198 makes custom parameters work at all.UnifiedCommand.Cleanupnow clears before disposing. Recycled commands keep their parameters, as before — recycling doesn't pass throughCleanup.