Fix transaction leak #2310
Merged
Fix transaction leak #2310
Conversation
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
| @@ -370,6 +370,7 @@ protected virtual void Dispose(bool isDisposing) | |||
| // don't dispose of multiple times. | |||
| return; | |||
| } | |||
| _isAlreadyDisposed = true; | |||
There was a problem hiding this comment.
Change kept from #2144: flagging it at the end of this method instead of as soon as possible causes the method to be vulnerable to an endless loop, in case a Dispose is performed inside AfterTransaction, called later by this method.
This change was required by #2144 since it was calling Dispose in AfterTransaction. This PR here does not do it and so does not actually need that change, but still it is safer to have it.
bahusoid
reviewed
Feb 4, 2020
fredericDelaporte
force-pushed
the
AdoTransactionLeak
branch
from
f96692a to
e0f606a
Compare
bahusoid
reviewed
Feb 4, 2020
bahusoid
reviewed
Feb 4, 2020
bahusoid
reviewed
Feb 6, 2020
bahusoid
reviewed
Feb 6, 2020
fredericDelaporte
force-pushed
the
AdoTransactionLeak
branch
from
02ce4bb to
21cf2bb
Compare
fredericDelaporte
force-pushed
the
AdoTransactionLeak
branch
from
21cf2bb to
b779aee
Compare
This comment has been minimized.
This comment has been minimized.
fredericDelaporte
force-pushed
the
AdoTransactionLeak
branch
from
b779aee to
04afb3e
Compare
To be squashed
fredericDelaporte
force-pushed
the
AdoTransactionLeak
branch
from
04afb3e to
443b462
Compare
|
Rebased for checking the generator update was not requiring a regen for this PR. |
bahusoid
approved these changes
Apr 27, 2020
|
I did a merge as 2 commits have better representation of the work done. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #2144
Instead of disposing
ConnectionManager._transactioninAfterTransactionas proposed in #2144, this obsoletesConnectionManager.Transactionin favor of a newConnectionManager.CurrentTransactionproperty which does not create a transaction when none are already created, but yieldsnull.So now any existing transaction should be the result of a call to
BeginTransaction, and its disposal is always a responsibility of the caller.