Codereview request for 6898310: (cs) Charset cache lookups should be synchronized (original) (raw)
Xueming Shen xueming.shen at oracle.com
Fri Sep 2 16:38:22 UTC 2011
- Previous message: Codereview request for 6898310: (cs) Charset cache lookups should be synchronized
- Next message: Codereview request for 6898310: (cs) Charset cache lookups should be synchronized
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On 09/02/2011 06:00 AM, Rémi Forax wrote:
On 09/02/2011 01:34 PM, Alan Bateman wrote:
Rémi Forax wrote:
Arghhh, next() can return null ! CharsetProvider provider = ... Iterator it = provider.charsets(); Iterator 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().
deleteCharset() was added later to workaround an "unusual" request for some Japanese charsets, and the AbstractcharsetProvider was shared by the ext and standard charset provider back then, so the "hook" was added in. In normal case, the provider is really not supposed to add a bunch of charsets during the initialization and then oops, I would like to delete some of them. So deleteCharset() is really not supposed to be used by anyone else, look it as an ugly implementation detail. Given the standard charset no longer uses the AbstractCharsetProvider, it might be the time to consider to have a clean implementation for the extended charset provider in JDK8. Yes, this is on the jdk8 to-do list.
-Sherman
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
- Previous message: Codereview request for 6898310: (cs) Charset cache lookups should be synchronized
- Next message: Codereview request for 6898310: (cs) Charset cache lookups should be synchronized
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]