Phil Steitz wrote:
> sebb wrote:
>> The method SharedPoolDataSource.getPooledConnectionAndInfo has the
>> following code:
>>
>> synchronized (userKeys) {
>> if (userKeys.containsKey(username)) {
>> userKeys.remove(username);
>> }
>> }
>>
>> Why not just use userKeys.remove(username)?
>
> That would work, other than other problem below.
>> What is the purpose of checking the map first?
>> Also, the keys used for userKeys consist of the username+password, so
>> this doesn't really make sense anyway - it will only remove an entry
>> if the password is the empty string.
>
> Yes, it should be username+password, so the code is really doing
> nothing now. The concatenation setup, btw, is bogus as there is no
> guarantee that you won't have collisions. Probably worth raising a
> defect for this.
The username+password key was introduced in the fix for DBCP-8. The
cleanup code was introduced in the fix for DBCP-245 (at which time
"username" by itself was the right key). The fix for DBCP-8 makes
the lack of cleanup no longer cause the bug in DBCP-245 to manifest.
Should still be fixed, though. I am on the fence on whether or not
to reopen DBCP-8. Likelihood of collision is low, but it is possible.
Phil
>
> Good catch, I will fix the cleanup bug.
>> Note that the synch. block may still be needed, because
>> LRUMap.remove() - i.e. SequencedHashMap.remove() - updates the
>> underlying map and some other fields but is not synchronised.
>> Unfortunately the Javadoc does not say what the class invariants are.
>
> Yes the synch is needed.
>
> Phil
>
>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
>> For additional commands, e-mail: dev-h...@commons.apache.org
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
