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

[Aleksey Shipilev]

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)

-- 
Thanks,
-Aleksey