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

[Ulf Zibis]

Am 02.09.2011 13:34, schrieb Alan Bateman:
> 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.
>>
There is another sync nit on lookups. See the comments on: 
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6853689


> deleteCharset isn't a public method so I don't think this can happen. I don't disagree that that 
> the iterator code should be re-written, just thinking it's separate from fixing the race condition.

Yes, and there are more reasons to rewrite the AbstractCharsetProvider:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6850361
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6862160
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6862165
https://bugs.openjdk.java.net/show_bug.cgi?id=100098

Instead _recoding_ an iterator for method charsets() you could simply use 
Collections.unmodifiableSet().iterator().
See example (Line 141 vs. 62):
https://bugs.openjdk.java.net/attachment.cgi?id=131&action=diff#a/src/share/classes/sun/nio/cs/FastCharsetProvider.java_sec2
For AbstractCharsetProvider (here renamed to ExternalCharsetProvider) See (Line 134):
https://bugs.openjdk.java.net/attachment.cgi?id=131&action=diff#a/src/share/classes/sun/nio/cs/ExternalCharsetProvider.java_sec1

-Ulf