Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed race condition connecting to the wrong SQLServer host in configurable IPv6 #1965

Merged
merged 6 commits into from
Dec 14, 2022
Merged

Fixed race condition connecting to the wrong SQLServer host in configurable IPv6 #1965

merged 6 commits into from
Dec 14, 2022

Conversation

mmimica
Copy link
Contributor

@mmimica mmimica commented Nov 11, 2022

Fix #1964 by not having a static field at all. It was cleared each time anyway.

ulvii and others added 6 commits December 3, 2020 12:37
@ulvii
Merge pull request microsoft#1471 from microsoft/dev
9e76aba 
Release | Merge dev to master for 9.1.1-preview release
@peterbae
Merge dev to master for 9.2.0 release (microsoft#1506)
66ee808 
Merge dev to master for 9.2.0 release (microsoft#1506)
Merge remote-tracking branch 'origin/main'
5788959 
# Conflicts:
#	CHANGELOG.md
#	README.md
#	azure-pipelines.yml
#	build.gradle
#	mssql-jdbc_auth_LICENSE
#	pom.xml
#	src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java
#	src/main/java/com/microsoft/sqlserver/jdbc/SQLJdbcVersion.java
#	src/main/java/com/microsoft/sqlserver/jdbc/SQLServerConnection.java
#	src/main/java/com/microsoft/sqlserver/jdbc/SQLServerMSAL4JUtils.java
#	src/samples/adaptive/pom.xml
#	src/samples/alwaysencrypted/pom.xml
#	src/samples/azureactivedirectoryauthentication/pom.xml
#	src/samples/connections/pom.xml
#	src/samples/constrained/pom.xml
#	src/samples/dataclassification/pom.xml
#	src/samples/datatypes/pom.xml
#	src/samples/resultsets/pom.xml
#	src/samples/sparse/pom.xml
#	src/test/java/com/microsoft/sqlserver/jdbc/TestResource.java
#	src/test/java/com/microsoft/sqlserver/jdbc/databasemetadata/DatabaseMetaDataTest.java
#	src/test/java/com/microsoft/sqlserver/jdbc/fedauth/ErrorMessageTest.java
fix merge
81395b1
Fix microsoft#1964 by not having a static field at all. It was cleare…
a9b907c 
…d each time anyway.
refactor a bit
4a67156
@lilgreenbird
Copy link
Contributor

thanks for the contribution @mmimica, could you please add a test for this?

@mmimica
Copy link
Contributor Author

mmimica commented Nov 15, 2022

I can't. A unit test would require a larger refactoring as host lookup code would need to be stubbed, and two servers in the test environment.
Besides, race-conditions are generally not reproducible. Correctness is assured by guarding the shared state (to be fair, there was an attempt there to guard it with a lock, but the lock is too fine-grained to be effective) and by not having needles shared state in the first place.

@lilgreenbird lilgreenbird added this to the 12.2.0 milestone Dec 8, 2022
@tkyc tkyc merged commit 128be37 into microsoft:main Dec 14, 2022
tkyc pushed a commit that referenced this pull request Dec 14, 2022
@mmimica @ulvii
@peterbae @tkyc
Fix connecting to the wrong SQLServer host (#1965)
a055204 
* Merge dev to master for 9.2.0 release (#1506)

Merge dev to master for 9.2.0 release (#1506)

* fix merge

* Fix #1964 by not having a static field at all. It was cleared each time anyway.

* refactor a bit

Co-authored-by: ulvii <v-ulibra@microsoft.com>
Co-authored-by: Peter Bae <v-hyba@microsoft.com>
Co-authored-by: Milan Mimica <milan.mimica@infobip.com>
@tkyc tkyc mentioned this pull request Dec 14, 2022
Merged
tkyc pushed a commit that referenced this pull request Dec 28, 2022
@mmimica @ulvii
@peterbae @tkyc
Fix connecting to the wrong SQLServer host (#1965)
e3594df 
* Merge dev to master for 9.2.0 release (#1506)

Merge dev to master for 9.2.0 release (#1506)

* fix merge

* Fix #1964 by not having a static field at all. It was cleared each time anyway.

* refactor a bit

Co-authored-by: ulvii <v-ulibra@microsoft.com>
Co-authored-by: Peter Bae <v-hyba@microsoft.com>
Co-authored-by: Milan Mimica <milan.mimica@infobip.com>
@tkyc tkyc mentioned this pull request Dec 28, 2022
Merged
tkyc pushed a commit that referenced this pull request Dec 29, 2022
@mmimica @ulvii
@peterbae @tkyc
Fix connecting to the wrong SQLServer host (#1965)
95f70a6 
* Merge dev to master for 9.2.0 release (#1506)

Merge dev to master for 9.2.0 release (#1506)

* fix merge

* Fix #1964 by not having a static field at all. It was cleared each time anyway.

* refactor a bit

Co-authored-by: ulvii <v-ulibra@microsoft.com>
Co-authored-by: Peter Bae <v-hyba@microsoft.com>
Co-authored-by: Milan Mimica <milan.mimica@infobip.com>
tkyc added a commit that referenced this pull request Dec 29, 2022
@tkyc @mmimica
@ulvii @peterbae
Fix connecting to the wrong SQLServer host (#1965) (#2019)
7e24f08 
* Merge dev to master for 9.2.0 release (#1506)

Merge dev to master for 9.2.0 release (#1506)

* fix merge

* Fix #1964 by not having a static field at all. It was cleared each time anyway.

* refactor a bit

Co-authored-by: ulvii <v-ulibra@microsoft.com>
Co-authored-by: Peter Bae <v-hyba@microsoft.com>
Co-authored-by: Milan Mimica <milan.mimica@infobip.com>

Co-authored-by: Milan Mimica <milan.mimica@gmail.com>
Co-authored-by: ulvii <v-ulibra@microsoft.com>
Co-authored-by: Peter Bae <v-hyba@microsoft.com>
Co-authored-by: Milan Mimica <milan.mimica@infobip.com>
@lilgreenbird lilgreenbird changed the title Fix connecting to the wrong SQLServer host Fixed race condition connecting to the wrong SQLServer host in configurable IPv6 Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkyc tkyc tkyc approved these changes

@Jeffery-Wasty Jeffery-Wasty Jeffery-Wasty approved these changes

@lilgreenbird lilgreenbird lilgreenbird approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
12.2.0
Development

Successfully merging this pull request may close these issues.

The driver connects to the wrong SQLServer host
6 participants
@mmimica @lilgreenbird @Jeffery-Wasty @tkyc @ulvii @peterbae