[13] RFR (XS): 8225141: Better handling of classes in error state in fast class initialization checks

[David Holmes]

Hi Vladimir,

On 1/06/2019 6:53 am, Vladimir Ivanov wrote:
> http://cr.openjdk.java.net/~vlivanov/8225141/webrev.00/
> https://bugs.openjdk.java.net/browse/JDK-8225141
> 
> Fast class initialization checks don't properly handle classes in error 
> state when performed from (previously) initializing thread.
> 
> One way to fix it is to add one more fast path check 
> (InstanceKlass::_init_state == being_initialized) into the barrier, but 
> that would require significant changes, since both newly introduced 
> checks (JDK-8223213 [1]) and existing C1 checks should be changed.
> 
> What I propose is to set InstanceKlass::_init_thread only for the 
> duration when the klass is in being_initialized state and reset it back 
> to NULL when changing class state. It makes existing "_init_thread == 
> current_thread" check equivalent to "_init_state == being_initialized && 
> _init_thread == current_thread".

That seems reasonable. By clearing the init_thread we no longer consider 
this a recursive initialization by the current thread and take the slow 
path which will reveal the class is erroneous.

Looking at the fast init changes I'm unclear why these assertions are valid:

+void LIR_Assembler::clinit_barrier(ciMethod* method) {
+  assert(method->holder()->is_being_initialized() || 
method->holder()->is_initialized(),
+         "initialization should have been started");

+  if (C->clinit_barrier_on_entry()) {
+    assert(C->method()->holder()->is_being_initialized() || 
C->method()->holder()->is_initialized(),
+           "initialization should have been started");

I'm not sure how we get to these code fragments such that its guaranteed 
initialization must already have been initiated (and possibly completed) 
- can't this be the first time we've called a static method and so the 
current thread would become responsible for doing the initialization? 
And again what if the class is actually in an error state? This ties in 
to JDK-8225106.

Thanks,
David
-----

> Testing: hs-precheckin-comp, tier1-4
> 
> Best regards,
> Vladimir Ivanov
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8223213