2009/12/18 sebb <seb...@gmail.com>:
> On 18/12/2009, ma...@apache.org <ma...@apache.org> wrote:
>> Author: markt
>> Date: Fri Dec 18 18:42:09 2009
>> New Revision: 892341
>>
>> URL: http://svn.apache.org/viewvc?rev=892341&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=47930
>> Make swapIn thread safe so parallel requests for the same session don't
>> result in multiple session objects for one sesison.
>>
>> + /**
>> + * Sessions currently being swapped in and the associated locks
>> + */
>> + private Map<String,Object> sessionSwapInLocks =
>> + new HashMap<String,Object>();
>
> Could/should be final ...
>
1)
* quickly find that the session is already in sessions, use it and
* carry on.
This is only true if loading succeeds.
If loading fails, nothing will be found in sessions, and loading will
be retried. Not good. (Not fatal, but waste of time).
I think that we can differentiate the thread that created the lock
from the next ones, that obtained it from the map, and allow only the
first one to perform loading.
A CountDownLatch can be useful here.
2)
if (sessionSwapInLocks.containsKey(id)) {
swapInLock = sessionSwapInLocks.get(id);
can be replaced with
swapInLock = sessionSwapInLocks.get(id);
if (swapInLock == null) { ...
3) I agree with sebb's proposal to make the field final.
3) Modified lines were indented with tabs, instead of spaces.
Misprints:
s/trues/tries/
s/will re-creates/will re-create/
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
