[8] RFR: Shenandoah: JvmtiExport::weak_oops_do should not be entered by multiple threads

[Roman Kennke]

That looks good to me. Thanks!

Roman


> The followup on sh/jdk8 hangs. JvmtiExport::weak_oops_do should not be entered by multiple threads,
> as we seen here:
>   https://mail.openjdk.java.net/pipermail/shenandoah-dev/2019-October/010960.html
> 
> But there is a wrinkle in jdk8, which does JvmtiExport::weak_oops_do as part of
> JNIHandles::weak_oops_do. And while we do JNIHandles::weak_oops_do in one thread, another thread can
> do JvmtiExport::weak_oops_do as the other task, and everything might go kaboom.
> 
> The way out I see is to let sh/jdk8 code piggyback on JNIHandles::weak_oops_do. The alternative is
> to special-case the JvmtiExport::weak_oops_do in JNIHandles::weak_oops_do, but that would require
> walking JvmtiExport::weak_oops_do in ShenandoahRootProcessor::process_vm_roots. That method does
> piggyback on JNIHandles::weak_oops_do already.
> 
> If this changes in future, then verifier should catch the unvisited weak JVMTI roots.
> 
> I haven't been able to reproduce the failure, but the logic seems to hold here, so I propose to push
> it for Christopher to test.
> 
> Fix:
> 
> diff -r 353f967c213d src/share/vm/gc_implementation/shenandoah/shenandoahRootProcessor.cpp
> --- a/src/share/vm/gc_implementation/shenandoah/shenandoahRootProcessor.cpp     Tue Aug 13 14:59:29
> 2019 +0200
> +++ b/src/share/vm/gc_implementation/shenandoah/shenandoahRootProcessor.cpp     Thu Oct 31 12:34:06
> 2019 +0100
> @@ -294,12 +294,18 @@
>    }
> 
>    if (!_evacuation_tasks->is_task_claimed(SHENANDOAH_EVAC_jvmti_oops_do)) {
>      ShenandoahWorkerTimingsTracker timer(worker_times, ShenandoahPhaseTimings::JVMTIRoots, worker_id);
>      JvmtiExport::oops_do(oops);
> +    // In JDK 8, this is handled by JNIHandles::weak_oops_do. We cannot enter here, because
> +    // it would walk the JvmtiTagMap twice (which is okay) and possibly by multiple threads
> +    // (which is not okay, because that walk is not thread-safe). In subsequent releases,
> +    // it is handled in a more straightforward manner, see JDK-8189360.
> +    /*
>      ShenandoahForwardedIsAliveClosure is_alive;
>      JvmtiExport::weak_oops_do(&is_alive, oops);
> +    */
>    }
> 
>    if (!_evacuation_tasks->is_task_claimed(SHENANDOAH_EVAC_SystemDictionary_oops_do)) {
>      ShenandoahWorkerTimingsTracker timer(worker_times,
> ShenandoahPhaseTimings::SystemDictionaryRoots, worker_id);
>      SystemDictionary::oops_do(oops);
> 
> 
> Testing: {x86_32, x86_64} hotspot_gc_shenandoah; shenandoah-bug reproducer (with verification)
>