Fix ActivityID map getting not cleaned up properly #1018
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #1018 +/- ##
============================================
- Coverage 50.16% 50.13% -0.04%
+ Complexity 2885 2880 -5
============================================
Files 120 120
Lines 27989 27992 +3
Branches 4677 4676 -1
============================================
- Hits 14040 14033 -7
- Misses 11693 11700 +7
- Partials 2256 2259 +3
Continue to review full report at Codecov.
|
|
mssql-jdbc-7.3.1-SNAPSHOT.jre11-preview-PR1018.jar.zip |
| @@ -33,7 +31,7 @@ static ActivityId getCurrent() { | |||
|
|
|||
| // Since the Id for each thread is unique, this assures that the below if statement is run only once per thread. | |||
| if (!activityIdTlsMap.containsKey(uniqueThreadId)) { | |||
| activityIdTlsMap.put(uniqueThreadId, new ActivityId()); | |||
| activityIdTlsMap.put(uniqueThreadId, new ActivityId(uniqueThreadId)); | |||
There was a problem hiding this comment.
Anywhere that calls getCurrent() is going to end up putting an entry in the map in the context of the current thread. In a pooled environment, each thread that uses the connection could potentially do that and then only the pooling thread that closes the connection will remove the original thread's entry from the map. There will still be a leak.
There was a problem hiding this comment.
Thanks. I'm looking into the documentation as to if Activity IDs are tied to a connection (one ActivityID per connection) or if it's tied to other queries as well. Depending on that, I think we can come up with a solution that doesn't affect the driver's performance - I'll come up with an update soon.
|
Continued in PR #1020. |
As mentioned in PR #465, the ActivityID objects in activityIdTlsMap would not get cleaned up in scenarios where there is a separate thread that manages the pooled connections. This is because the key that is used to look up and remove the entries in the map comes from the current thread's ID, and in a scenario where a separate thread is responsible for cleaning up all the threads, the thread's ID will stay constant throughout the rest of the application's lifetime and the ActivityIDs will not get cleaned up properly.
To solve this issue, instead of relying on recomputing the current thread's ID when we need to clean up the map, we associate each SQLServerConnection object with the thread ID, and use that variable later when we call close() on the connection. This will ensure that the map is getting cleaned up properly in all scenarios.