On Mon, 4 Nov 2024 03:08:02 GMT, Ioi Lam <ik...@openjdk.org> wrote:

>> src/hotspot/share/cds/aotClassInitializer.cpp line 348:
>> 
>>> 346:     }
>>> 347:     JavaValue result(T_VOID);
>>> 348:     JavaCalls::call_static(&result, ik,
>> 
>> Based on the discussions in JDK-8342283, do we need a memory fence after the 
>> call to runtimeSetup?
>> 
>> I think we do to be consistent with the non-AOTCache run as there is a fence 
>> after `<clinit>` (when this code would normally run) due to the 
>> initialization lock.
>
> I think this is already done:
> 
> `runtimeSetup()` is called inside `InstanceKlass::call_class_initializer()`. 
> When that returns, we will proceed to 
> `InstanceKlass::set_initialization_state_and_notify()` which will perform the 
> memory fencing. 
> 
> https://github.com/iklam/jdk/blob/6eebd18fc2820ffb179d431f179fc6af6d1be247/src/hotspot/share/oops/instanceKlass.cpp#L1276-L1300

I think you're right about the memory fencing being covered there but reading 
that code shows another potential issue - what if the `runtimeSetup()` call 
throws an exception?

If I read `InstanceKlass::call_class_initializer()` correctly, an exception 
from `runtimeSetup()` will lead to marking the class as failing initialization 
(`initialization_error`) which will be an unexpected outcome as there may be 
instances of the class in the AOTCache that aren't produced by paths through 
the `<clinit>` method.

Classes are able to create an instance in `<clinit>` and escape it to other 
code and then fail initialization.  The instances are left in an odd state 
where after the `<clinit>` the instance methods can be called but static fields 
and static methods can't be accessed.

`runtimeSetup()` expands on this corner case which I think is OK for current 
"crawl" stage of our "crawl-walk-run" approach as its analogous to the existing 
`<clinit>` behaviour.  It may need refining in the future to abort the VM or do 
something else (?) in the case of failed `runtimeSetup` calls

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21642#discussion_r1827813351

Reply via email to