Request for review: Race conditions in java.nio.charset.Charset

[Xueming Shen]

Ulf,

Though I did not write the cache code my reading suggests the existing cache
impl tries to avoid the relatively "expensive" synchronization for most 
use scenarios,
your approach however hits the synchronization for each/every lookup 
invocation.
So you should at least consider to put the sync block in a separate 
lookup2.

But personally I doubt we really care this "race condition", it's a 
cache, a cache miss
is not a disaster, in fact in most cases, since the charset has been in 
the cache
already, it should be in the providers' "cache" as well,  especially for 
the bundled
standard & extended charset provider, the lookup actually is 1 or 2  
O(1) hashmap
lookup (yes, an additional name canonicalization and the "expensive" sync).

So maybe we can do something like

b = cache1;
cache1 = a;
cache2 = b;

to make the situation a little better, or maybe worse:-)

sherman


sherman

Ulf Zibis wrote:
> I.) Internal charset cache will be corrupted in theoretical race 
> conditions:
>
> Startpoint:
>  cache1 --> Charset1
>  cache2 --> Charset2
>
> Scenario 1:
> - Thread1 asks for Charset2 via Charset.forName("Charset2").
> - If Thread1 is interrupted after code line: cache2 = cache1
>  in method lookup2(String charsetName) we have:
>  cache1 --> Charset1
>  cache2 --> Charset1
>> If now thread2 asks for Charset2, it can't find it in cache, so it 
>> invokes
>  an expensive lookup via registered providers.
>
> Scenario 2:
> - Thread1 asks for Charset2.
> - Thread1 is interupted before cache2 = cache1;
> - If now thread2 asks for Charset2 we have after completing lookup2():
>  cache1 --> Charset2
>  cache2 --> Charset1
> - Now thread1 resumes; after completing lookup2() we have:
>  cache1 --> Charset2
>  cache2 --> Charset2
>> In the end we can see, that Charset1 is lost in cache.
>
>
> II.) Endless loop, if VM's default charset (called by 
> Charset.defaultCharset())
>     needs to load mapping data via Class.getResourceAsStream():
>
> - Invoking Class.getResourceAsStream(), again calls for 
> Charset.defaultCharset(),
>  so we are in endless loop.
>
> Note: The error condition, described in
>      http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6795536
>      is fixed since Bug ID 6797688, but the endless loop, getting the 
> default charset, remains.
>
>
> See my patches here: https://bugs.openjdk.java.net/show_bug.cgi?id=100107
>
>
> -Ulf
>
>
>