Codereview request for 6898310: (cs) Charset cache lookups should be synchronized

Rémi Forax forax at univ-mlv.fr
Fri Sep 2 13:00:38 UTC 2011


On 09/02/2011 01:34 PM, Alan Bateman wrote:
> Rémi Forax wrote:
>>
>> Arghhh, next() can return null !
>>
>> CharsetProvider provider = ...
>> Iterator<Charset> it = provider.charsets();
>> Iterator<Charset> it2 = provider.charsets();
>> Charset charset = it.next();
>> provider.deleteCharset(charset.name(), ...)
>> System.out.println(it2.next());   // print null
>>
>> even if I'm not sure a lot of CharsetProvider actually calls 
>> deleteCharset
>> there is a possible bug here.
>>
> deleteCharset isn't a public method so I don't think this can happen.

I doesn't happen currently in the JDK because deleteCharset() (and 
charset())
are only called in ExtendedCharset.init().

> I don't disagree that that the iterator code should be re-written, 
> just thinking it's separate from fixing the race condition.

A way to solve the problem is to split AbstractCharsetProvider in two 
objects
i.e we should create a new object named CharsetProviderView that contains
deleteCharset() and charset() and provide this object as parameter of init.

The idea is that during the initialization (in init()) calling 
deleteCharset/charset is safe,
not after.

>
> -Alan.
>

Rémi




More information about the core-libs-dev mailing list