issue#2537 - Fix for SQLServerConnection infinite loop#2547
Merged
Conversation
machavan
requested review from
David-Engel,
Jeffery-Wasty,
codewatchzen,
divang,
lilgreenbird and
tkyc
Contributor
Author
|
Testing:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2547 +/- ##
=========================================
Coverage 50.96% 50.96%
- Complexity 3912 3915 +3
=========================================
Files 147 147
Lines 33460 33462 +2
Branches 5608 5607 -1
=========================================
+ Hits 17052 17055 +3
- Misses 13999 14000 +1
+ Partials 2409 2407 -2 ☔ View full report in Codecov by Sentry. |
divang
reviewed
Nov 26, 2024
divang
approved these changes
Nov 26, 2024
tkyc
approved these changes
Nov 26, 2024
Jeffery-Wasty
approved these changes
Nov 26, 2024
Contributor
|
@machavan You need to agree to the CLA in the following format:
Before we can merge the PR. |
Contributor
Author
|
@microsoft-github-policy-service agree [company="{Microsoft}"] |
Contributor
Author
|
@microsoft-github-policy-service agree company="Microsoft" |
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
5 participants
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.
Description:
Based on issue analysis
"Could be looping infinitely because bIsClosed could be true. bIsClosed could have been set to true earlier somewhere."
we are improving the infinite loop to close all statements present in openStatements , remove them from it explicitly and terminate.
Also, it looks like this could be happening as connections/statements are being used across threads based on the issue description.
bIsClosed and openStatements are not volatile, and as a result a thread creating statements while under a request ( started using beginRequest) may add them in openStatements list, with some other thread closing them but not removing from openStatements just due to that thread having read a stale value(null) of openStatements( and other combinations of issues of this sort). Hence, we are making bIsClosed and openStatements volatile.
Testing: