RFR 8012326: Deadlock occurs when Charset.availableCharsets() is called by several threads at the same time

[Xueming Shen]

Hi,

Please help review the proposed fix for 8012326.

The direct trigger is the fix we put in #6898310 [1], in which we added the
synchronization to prevent a possible race condition as described in 
$4685305.

However it appears the added synchronization triggers another race 
condition as
showed in this stack trace [4] when run the test case [2][3].

The root cause here is

(1) The ExtendedCharsetProvider is intended to be used as a singleton 
(as the
probeExtendedProvider + lookupExtendedCharset mechanism in Charset.java),
however this is only true for the 
Charset.forName()->lookup()...scenario. Multiple
instances of ExtendedCharsetProvider is being created in 
Charset.availableCharset()
every time it is invoked, via providers()/ServiceLoader.load().

(2) ISO2022_JP_2 and MSISO2022JP are provided via ExtendedCharsetProvider,
however both of them have two static variable DEC02{12|08}/ENC02{12|08} that
need to be initialized during the "class initialization", which will 
indirectly trigger
the invocation of ExtendedCharsetProvider.alisesFor(...)

(3) static method ExtendedCharsets.aliaseFor() has a hacky 
implementation that
goes to AbstractCharsetProvider.alise() for lookup, which needs to 
obtain a lock
on its ExtendedCharesetProvider.instance. This will potential cause race 
condition
if the "instance" it tries to synchronize on is not its "own" instance. 
This is possible
because the constructor of ExtendedCharsetProvider overwrites static 
"instance"
variable with "this".

Unfortunately as demonstrated  in [4], this appears to be what is happening.
The Thread-1/#9 is trying to synchronize on someone else's 
ExtendedCharsetProvider
instance <0xa4824730> (its own instance should be <0xa4835328>, which it
holds the monitor in the iterator.next()), it is blocked because 
Thread-0 already holds
the monitor of <0xa4824730> (in its iteratior.next()), but Thread-0 is 
blocked at
Class.forName0(), which I think is because Thread-1 is inside it 
already...two theads
are deadlocked.

A "quick fix/solution" is to remove the direct trigger in ISO2022_JP_2 and
MSISO2022JP to lazily-initialize those static variables as showed in the
webrev. However while this probably will solve the race condition, the 
multiple
instances of ExtendedCharsetProvider is really not desired. And given the
fact we have already hardwired ExtendedCharsetProvider (as well as the
StandardCharset, for performance reason) for charset lookup/forName(), the
availableCharsets() should also utilize the "singleton" 
ExtendedCharsetProvider
instanc (extendedCharsetProvider) as well, instead of going to the 
ServiceLoader
every time for a new instance. The suggested change is to use Charset.
extendedCharsetProvide via probeExtendedProvider() for extended charset
iteration (availableCharset()). Meanwhile, since the 
ExtendedCharsetProvider/
charsets.jar should always be in the boot classpath (if installed, and 
we are
looking up via Class.forName("sun.nio.cs.ext.ExtededCharsetProvider")), 
there
is really no need for it to be looked up/loaded via the spi mechanism (in
fact it's redundant to load it again after we have lookup/iterate the
hardwired "extendedCharsetProvider". So I removed the spi description from
the charsts.jar as well.

An alternative is to really implement the singleton pattern in ECP, however
given the existing mechanism we have in Charset.java and the "hacky" init()
implementation we need in ECP, I go with the current approach.

http://cr.openjdk.java.net/~sherman/8012326/webrev

Thanks,
Sherman

[1] http://cr.openjdk.java.net/~sherman/6898310/webrev/
[2] http://cr.openjdk.java.net/~sherman/8012326/runtest.bat
[3] http://cr.openjdk.java.net/~sherman/8012326/Test.java
[4] http://cr.openjdk.java.net/~sherman/8012326/stacktrace