Fix shared timer race #2084 #2085
Conversation
Release | Merge dev to master for 9.1.1-preview release
Merge dev to master for 9.2.0 release (microsoft#1506)
# 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
|
After reviewing the PR again and talking with the team, I agree with the code changes, but I'm personally having issues running the test. I think it may just be my environment and will get back to you if when I can confirm everything is working correctly. |
There was a problem hiding this comment.
Not sure if this actually waits for an approval and if I'm the right person to approve, but still: Looks fine 👍
I was able to reproduce the race condition (an NPE was thrown) with the test before the fix and can verify that the NPE is gone after the fix is applied.
I also wrote an extended version of the test, using a latch to make sure all threads start at the same time. But as it turned out your test works fine for reproducing the issue.
(Due to security manager issues I just made a local copy of SharedTime and used that.)
|
Hi @jubax, Yes, we were just making sure the test was passing consistently. I was having issues on my end, but that has been resolved. Everything looks fine, we'll look at getting this merged. |
No description provided.