8228490: Shenandoah: Shenandoah concurrent root evacuation may race against OopStorage::release()
Reviewed-by: rkennke
--- a/src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp Thu Jul 25 12:20:04 2019 +0530
+++ b/src/hotspot/share/gc/shenandoah/shenandoahClosures.hpp Mon Jul 22 16:32:58 2019 -0400
@@ -79,6 +79,16 @@
inline void do_oop_work(T* p);
};
+class ShenandoahEvacUpdateOopStorageRootsClosure : public BasicOopIterateClosure {
+private:
+ ShenandoahHeap* _heap;
+ Thread* _thread;
+public:
+ inline ShenandoahEvacUpdateOopStorageRootsClosure();
+ inline void do_oop(oop* p);
+ inline void do_oop(narrowOop* p);
+};
+
#ifdef ASSERT
class ShenandoahAssertNotForwardedClosure : public OopClosure {
private:
--- a/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp Thu Jul 25 12:20:04 2019 +0530
+++ b/src/hotspot/share/gc/shenandoah/shenandoahClosures.inline.hpp Mon Jul 22 16:32:58 2019 -0400
@@ -107,6 +107,31 @@
do_oop_work(p);
}
+ShenandoahEvacUpdateOopStorageRootsClosure::ShenandoahEvacUpdateOopStorageRootsClosure() :
+ _heap(ShenandoahHeap::heap()), _thread(Thread::current()) {
+}
+
+void ShenandoahEvacUpdateOopStorageRootsClosure::do_oop(oop* p) {
+ assert(_heap->is_evacuation_in_progress(), "Only do this when evacuation is in progress");
+
+ oop obj = RawAccess<>::oop_load(p);
+ if (! CompressedOops::is_null(obj)) {
+ if (_heap->in_collection_set(obj)) {
+ shenandoah_assert_marked(p, obj);
+ oop resolved = ShenandoahBarrierSet::resolve_forwarded_not_null(obj);
+ if (oopDesc::equals_raw(resolved, obj)) {
+ resolved = _heap->evacuate_object(obj, _thread);
+ }
+
+ Atomic::cmpxchg(resolved, p, obj);
+ }
+ }
+}
+
+void ShenandoahEvacUpdateOopStorageRootsClosure::do_oop(narrowOop* p) {
+ ShouldNotReachHere();
+}
+
#ifdef ASSERT
template <class T>
void ShenandoahAssertNotForwardedClosure::do_oop_work(T* p) {
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp Thu Jul 25 12:20:04 2019 +0530
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp Mon Jul 22 16:32:58 2019 -0400
@@ -1593,12 +1593,19 @@
void work(uint worker_id) {
ShenandoahEvacOOMScope oom;
- ShenandoahEvacuateUpdateRootsClosure cl;
- CLDToOopClosure clds(&cl, ClassLoaderData::_claim_strong);
-
- _jni_roots.oops_do<ShenandoahEvacuateUpdateRootsClosure>(&cl);
- _cld_roots.cld_do(&clds);
- _weak_roots.oops_do<ShenandoahEvacuateUpdateRootsClosure>(&cl);
+ {
+ // jni_roots and weak_roots are OopStorage backed roots, concurrent iteration
+ // may race against OopStorage::release() calls.
+ ShenandoahEvacUpdateOopStorageRootsClosure cl;
+ _jni_roots.oops_do<ShenandoahEvacUpdateOopStorageRootsClosure>(&cl);
+ _weak_roots.oops_do<ShenandoahEvacUpdateOopStorageRootsClosure>(&cl);
+ }
+
+ {
+ ShenandoahEvacuateUpdateRootsClosure cl;
+ CLDToOopClosure clds(&cl, ClassLoaderData::_claim_strong);
+ _cld_roots.cld_do(&clds);
+ }
}
};