On Mon, 4 Nov 2024 14:21:09 GMT, Dan Heidinga <heidi...@openjdk.org> wrote:

>> 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

Thanks for catching this. I added code to exit the JVM when `runtimeSetup()` 
fails.

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

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

Reply via email to