changeset 57796:082f1d3eb164

8235305: Corrupted oops embedded in nmethods due to parallel modification during optional evacuation Summary: During optional evacuation it is possible that G1 modifies oops embedded in nmethods in parallel. One source are oop* gathered by a previous evacuation phase in the optional roots, the other the region's strong code roots list. Since these oops may be unaligned on x64, this can result in them being corrupted. The fix is to not gather embedded oops in the optional roots list as the strong code roots list contains them already. Reviewed-by: sjohanss, stefank Contributed-by: erik.osterlund@oracle.com, stefan.johansson@oracle.com, stefan.karlsson@oracle.com, thomas.schatzl@oracle.com
author tschatzl
date Wed, 22 Jan 2020 10:00:13 +0100
parents 5dae0969decc
children 91ea567eeabe
files src/hotspot/share/gc/g1/g1OopClosures.hpp src/hotspot/share/gc/g1/g1OopClosures.inline.hpp src/hotspot/share/gc/g1/g1SharedClosures.hpp
diffstat 3 files changed, 13 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/gc/g1/g1OopClosures.hpp	Thu Jan 16 16:30:20 2020 -0500
+++ b/src/hotspot/share/gc/g1/g1OopClosures.hpp	Wed Jan 22 10:00:13 2020 +0100
@@ -152,7 +152,8 @@
 
 enum G1Barrier {
   G1BarrierNone,
-  G1BarrierCLD
+  G1BarrierCLD,
+  G1BarrierNoOptRoots  // Do not collect optional roots.
 };
 
 enum G1Mark {
--- a/src/hotspot/share/gc/g1/g1OopClosures.inline.hpp	Thu Jan 16 16:30:20 2020 -0500
+++ b/src/hotspot/share/gc/g1/g1OopClosures.inline.hpp	Wed Jan 22 10:00:13 2020 +0100
@@ -246,7 +246,7 @@
   } else {
     if (state.is_humongous()) {
       _g1h->set_humongous_is_live(obj);
-    } else if (state.is_optional()) {
+    } else if ((barrier != G1BarrierNoOptRoots) && state.is_optional()) {
       _par_scan_state->remember_root_into_optional_region(p);
     }
 
--- a/src/hotspot/share/gc/g1/g1SharedClosures.hpp	Thu Jan 16 16:30:20 2020 -0500
+++ b/src/hotspot/share/gc/g1/g1SharedClosures.hpp	Wed Jan 22 10:00:13 2020 +0100
@@ -40,6 +40,14 @@
 public:
   G1ParCopyClosure<G1BarrierNone, Mark> _oops;
   G1ParCopyClosure<G1BarrierCLD,  Mark> _oops_in_cld;
+  // We do not need (and actually should not) collect oops from nmethods into the
+  // optional collection set as we already automatically collect the corresponding
+  // nmethods in the region's strong code roots set. So set G1BarrierNoOptRoots in
+  // this closure.
+  // If these were present there would be opportunity for multiple threads to try
+  // to change this oop* at the same time. Since embedded oops are not necessarily
+  // word-aligned, this could lead to word tearing during update and crashes.
+  G1ParCopyClosure<G1BarrierNoOptRoots, Mark> _oops_in_nmethod;
 
   G1CLDScanClosure                _clds;
   G1CodeBlobClosure               _codeblobs;
@@ -47,6 +55,7 @@
   G1SharedClosures(G1CollectedHeap* g1h, G1ParScanThreadState* pss, bool process_only_dirty) :
     _oops(g1h, pss),
     _oops_in_cld(g1h, pss),
+    _oops_in_nmethod(g1h, pss),
     _clds(&_oops_in_cld, process_only_dirty),
-    _codeblobs(pss->worker_id(), &_oops, needs_strong_processing()) {}
+    _codeblobs(pss->worker_id(), &_oops_in_nmethod, needs_strong_processing()) {}
 };