On 14 October 2010 00:05, Konstantin Kolinko <[email protected]> wrote:
> 2010/10/13  <[email protected]>:
>> Author: markt
>> Date: Wed Oct 13 14:41:57 2010
>> New Revision: 1022117
>>
>> URL: http://svn.apache.org/viewvc?rev=1022117&view=rev
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=49987
>> Thread safety issue with population of servlet context initialisation 
>> parameters.
>> Back-port wasn't completely clean - please review.
>>
>
> +1. Looks OK for me.
>
> Re: sebb:
> 2010/10/13 sebb <[email protected]>:
>>> -    private Map parameters = null;
>>> +    private Map<String,String> parameters =
>>> +        new ConcurrentHashMap<String,String>();
>>
>> This should be final, surely?
>>
>
> There are a lot of non-final fields in ApplicationContext and I think
> in related classes as well. I do not think making this specific one as
> final is essential.  (Anyway, +1 if anybody wants to do that).

Given that the fix is for thread-safety, using final is intended to
ensure that the field is correctly published following construction.
There's little point in using the thread-safe ConcurrentHashMap if the
field is not identical in all threads...

> Best regards,
> Konstantin Kolinko
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to