RFR: 8274338: com/sun/jdi/RedefineCrossEvent.java failed "assert(m != __null) failed: NULL mirror" [v3]

[Ioi Lam]

On Thu, 14 Oct 2021 22:24:53 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> I will state again:  If the JVMTI code is racing with the execution of restore_unshareable_info and the setting of the mirror, then even with the lock added the JVMTI code could still get there first and find the mirror not set.
>> 
>> The added lock simply ensures that the JVMTI code and the restore_unshareable_info code cannot execute concurrently - it does not guarantee that the JVMTI code can't execute until after restore_unshareable_info.
>
>> I will state again: If the JVMTI code is racing with the execution of restore_unshareable_info and the setting of the mirror, then even with the lock added the JVMTI code could still get there first and find the mirror not set.
>> 
>> The added lock simply ensures that the JVMTI code and the restore_unshareable_info code cannot execute concurrently - it does not guarantee that the JVMTI code can't execute until after restore_unshareable_info.
> 
> For an InstanceKlass k to be iterated on by ClassLoaderData::loaded_classes_do(), k->is_loaded() must be true. The class enters the "loaded" state when SystemDictionay::add_to_hierarchy(k) is called, which happens after k->restore_unshareable_info() has completed. So the current implementation is safe w.r.t. InstanceKlasses.
> 
> For ArrayKlasses, they would be *kind of* safe after this PR, as the JVMTI code will not see an ArrayKlasses that's in the middle of restore_unshareable_info(). However, it will still be possible for the JVMTI code to see the `[LFooBar;` class without seeing the `FooBar;` class. This may produce unexpected results.

> @iklam you're right - we'd see the ObjArrayKlass first since the InstanceKlass will be added to the CLD before its marked as is_loaded(). I don't know what the result of that would be. You could open a new bug for that to investigate that and try to write a test case (using JVMTI that would show if it got confused).

I filed https://bugs.openjdk.java.net/browse/JDK-8275318 "loaded_classes_do may see ArrayKlass before InstanceKlass is loaded"

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

PR: https://git.openjdk.java.net/jdk/pull/5935