RFR: 8308341: JNI_GetCreatedJavaVMs returns a partially initialized JVM [v2]

[Gerard Ziemski]

On Tue, 30 May 2023 14:39:21 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>>> I have one question though: why wouldn't it be enough to move `vm_created = 1` from where we had it before, down to where we now have `vm_created = COMPLETE` ?
>> 
>> Because we need to prevent two threads from concurrently loading and initializing a VM in the same process.
>
>> > I have one question though: why wouldn't it be enough to move `vm_created = 1` from where we had it before, down to where we now have `vm_created = COMPLETE` ?
>> 
>> Because we need to prevent two threads from concurrently loading and initializing a VM in the same process.
> 
> Wouldn't it be less complex to use a lock or some "init once" mechanism (pthread_once) instead of a 3 stage atomic field? Do we really need to know whether the process is in the middle of initialization for any reason other than whether it's actually done?
> 
> Just to clarify - I'm OK with the fix you have proposed, I was just curious if you considered any other alternatives.

>> Thanks for the review @gerard-ziemski
>> 
>> > Just to clarify - I'm OK with the fix you have proposed, I was just curious if you considered any other alternatives.
>> 
>> Atomics are the simplest cross-platform solution available at this stage of VM init (which is "nothing is initialized yet") - even >Atomic use is limited to Atomic::xchg as we can't require any stubs. We have no VM locks available and anything external to the >VM (like pthread_once) would require as OS-specific solution in shared code.

I guess it's a compromise either way. You have chosen to use an OS independent mechanism, at the cost of exposing the implementation to the outside world, by introducing a new stage (needs CSR). I think, I would have prefer (with what I know at this time) with keeping the implementation details hidden from outside (no need for CSR), at the cost of using platform dependent locks here, or some sort of initialize once mechanism.

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

PR Comment: https://git.openjdk.org/jdk/pull/14139#issuecomment-1570451028