RFR: 8293252: Shenandoah: ThreadMXBean synchronizer tests crash with aggressive heuristics [v2]

[Ashutosh Mehra]

> Please review the fix for the assertion failure when running ThreadMXBean synchronizer code in fastdebug mode.
> 
> Please note that the assertion failure happens in regular (satb) mode as well, not just in iu mode.
> 
> At the point of assertion failure, the JVM is at a safepoint as the VMThread is executing VM_ThreadDump operation and the GC worker threads are paused after the mark phase.
> Assertion happened when the VMThread performs `NativeAccess<>::oop_store()` (as part of creating an `OopHandle`) on an object which has been marked and is in the collection set. But the failing assertion in `oop_store_not_in_heap()` expects the oop is not in the collection set.
> 
> The assertion is conditional on the statement `value != NULL && !ShenandoahHeap::heap()->cancelled_gc()`
> 
> 	shenandoah_assert_not_in_cset_if(addr, value, value != NULL && !ShenandoahHeap::heap()->cancelled_gc());
> 
> But it looks like it is missing another important condition - the GC should be in marking phase.
> i.e. the assertion is valid only if it is protected by `ShenandoahHeap::heap()->is_concurrent_mark_in_progress()`.
> So the correct assertion should be:
> 
> 	shenandoah_assert_not_in_cset_if(addr, value, value != NULL && !ShenandoahHeap::heap()->cancelled_gc() && ShenandoahHeap::heap()->is_concurrent_mark_in_progress());
> 
> In fact this assertion is already present in one of its caller (`oop_store_in_heap()1) in its negative form:
> 
> 	shenandoah_assert_not_in_cset_except    (addr, value, value == NULL || ShenandoahHeap::heap()->cancelled_gc() || !ShenandoahHeap::heap()->is_concurrent_mark_in_progress());
> 
> 
> Also, it reads strange that `oop_store_in_heap()` would call `oop_store_not_in_heap()`.
> So I have moved the code to a separate method `oop_store_common()` that gets called by both `oop_store_in_heap()` and `oop_store_not_in_heap()`.
> 
> Tested it by running following tests in fastdebug mode:
> 
> - hotspot_gc_shenandoah
> - java/lang/management/ThreadMXBean/MyOwnSynchronizer.java (50 times with -XX:ShenandoahGCHeuristics=aggressive)
> - java/lang/management/ThreadMXBean/LockedSynchronizers.java (50 times with -XX:ShenandoahGCHeuristics=aggressive)
> 
> Signed-off-by: Ashutosh Mehra <asmehra at redhat.com>

Ashutosh Mehra has updated the pull request incrementally with two additional commits since the last revision:

 - Use HeapAccess instead of RawAccess when iterating the heap objects
   
   Signed-off-by: Ashutosh Mehra <asmehra at redhat.com>
 - Make oop_store_common() as private method and remove extraneous condition
   
   Signed-off-by: Ashutosh Mehra <asmehra at redhat.com>

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/10268/files
  - new: https://git.openjdk.org/jdk/pull/10268/files/41629506..ee36ab82

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=10268&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=10268&range=00-01

  Stats: 18 lines in 3 files changed: 4 ins; 9 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/10268.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/10268/head:pull/10268

PR: https://git.openjdk.org/jdk/pull/10268