7097053: G1: assert(da ? referent->is_oop() : referent->is_oop_or_null())...

7097053: G1: assert(da ? referent->is_oop() : referent->is_oop_or_null()) failed: referenceProcessor.cpp:1054
Summary: During remembered set scanning, the reference processor could discover a reference object whose referent was in the process of being copied and so may not be completely initialized. Do not perform reference discovery during remembered set scanning.
Reviewed-by: tonyp, ysr

src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp

@@ -4618,15 +4618,7 @@ public:
       scan_perm_cl = &scan_mark_perm_cl;
     }
 
-    // The following closure is used to scan RSets looking for reference
-    // fields that point into the collection set. The actual field iteration
-    // is performed by a FilterIntoCSClosure, whose do_oop method calls the
-    // do_oop method of the following closure.
-    // Therefore we want to record the reference processor in the
-    // FilterIntoCSClosure. To do so we record the STW reference
-    // processor into the following closure and pass it to the
-    // FilterIntoCSClosure in HeapRegionDCTOC::walk_mem_region_with_cl.
-    G1ParPushHeapRSClosure          push_heap_rs_cl(_g1h, &pss, rp);
+    G1ParPushHeapRSClosure          push_heap_rs_cl(_g1h, &pss);
 
     pss.start_strong_roots();
     _g1h->g1_process_strong_roots(/* not collecting perm */ false,

src/share/vm/gc_implementation/g1/g1OopClosures.hpp

@@ -61,13 +61,8 @@ public:
 class G1ParPushHeapRSClosure : public G1ParClosureSuper {
 public:
   G1ParPushHeapRSClosure(G1CollectedHeap* g1,
-                         G1ParScanThreadState* par_scan_state,
-                         ReferenceProcessor* rp) :
-    G1ParClosureSuper(g1, par_scan_state)
-  {
-    assert(_ref_processor == NULL, "sanity");
-    _ref_processor = rp;
-  }
+                         G1ParScanThreadState* par_scan_state):
+    G1ParClosureSuper(g1, par_scan_state) { }
 
   template <class T> void do_oop_nv(T* p);
   virtual void do_oop(oop* p)          { do_oop_nv(p); }
@@ -190,13 +185,8 @@ class FilterIntoCSClosure: public OopClosure {
 public:
   FilterIntoCSClosure(  DirtyCardToOopClosure* dcto_cl,
                         G1CollectedHeap* g1,
-                        OopClosure* oc,
-                        ReferenceProcessor* rp) :
-    _dcto_cl(dcto_cl), _g1(g1), _oc(oc)
-  {
-    assert(_ref_processor == NULL, "sanity");
-    _ref_processor = rp;
-  }
+                        OopClosure* oc) :
+    _dcto_cl(dcto_cl), _g1(g1), _oc(oc) { }
 
   template <class T> void do_oop_nv(T* p);
   virtual void do_oop(oop* p)        { do_oop_nv(p); }

src/share/vm/gc_implementation/g1/g1RemSet.cpp

@@ -468,7 +468,7 @@ public:
     MemRegion scanRegion(start, end);
 
     UpdateRSetImmediate update_rs_cl(_g1->g1_rem_set());
-    FilterIntoCSClosure update_rs_cset_oop_cl(NULL, _g1, &update_rs_cl, NULL /* rp */);
+    FilterIntoCSClosure update_rs_cset_oop_cl(NULL, _g1, &update_rs_cl);
     FilterOutOfRegionClosure filter_then_update_rs_cset_oop_cl(r, &update_rs_cset_oop_cl);
 
     // We can pass false as the "filter_young" parameter here as:
@@ -644,7 +644,7 @@ bool G1RemSet::concurrentRefineOneCard_impl(jbyte* card_ptr, int worker_i,
   update_rs_oop_cl.set_from(r);
 
   TriggerClosure trigger_cl;
-  FilterIntoCSClosure into_cs_cl(NULL, _g1, &trigger_cl, NULL /* rp */);
+  FilterIntoCSClosure into_cs_cl(NULL, _g1, &trigger_cl);
   InvokeIfNotTriggeredClosure invoke_cl(&trigger_cl, &into_cs_cl);
   Mux2Closure mux(&invoke_cl, &update_rs_oop_cl);
 

src/share/vm/gc_implementation/g1/heapRegion.cpp

@@ -210,44 +210,17 @@ void HeapRegionDCTOC::walk_mem_region_with_cl(MemRegion mr,
                                               HeapWord* top,
                                               OopClosure* cl) {
   G1CollectedHeap* g1h = _g1;
-
   int oop_size;
+  OopClosure* cl2 = NULL;
 
-  OopClosure* cl2 = cl;
-
-  // If we are scanning the remembered sets looking for refs
-  // into the collection set during an evacuation pause then
-  // we will want to 'discover' reference objects that point
-  // to referents in the collection set.
-  //
-  // Unfortunately it is an instance of FilterIntoCSClosure
-  // that is iterated over the reference fields of oops in
-  // mr (and not the G1ParPushHeapRSClosure - which is the
-  // cl parameter).
-  // If we set the _ref_processor field in the FilterIntoCSClosure
-  // instance, all the reference objects that are walked
-  // (regardless of whether their referent object's are in
-  // the cset) will be 'discovered'.
-  //
-  // The G1STWIsAlive closure considers a referent object that
-  // is outside the cset as alive. The G1CopyingKeepAliveClosure
-  // skips referents that are not in the cset.
-  //
-  // Therefore reference objects in mr with a referent that is
-  // outside the cset should be OK.
-
-  ReferenceProcessor* rp = _cl->_ref_processor;
-  if (rp != NULL) {
-    assert(rp == _g1->ref_processor_stw(), "should be stw");
-    assert(_fk == IntoCSFilterKind, "should be looking for refs into CS");
-  }
-
-  FilterIntoCSClosure intoCSFilt(this, g1h, cl, rp);
+  FilterIntoCSClosure intoCSFilt(this, g1h, cl);
   FilterOutOfRegionClosure outOfRegionFilt(_hr, cl);
 
   switch (_fk) {
+  case NoFilterKind:          cl2 = cl; break;
   case IntoCSFilterKind:      cl2 = &intoCSFilt; break;
   case OutOfRegionFilterKind: cl2 = &outOfRegionFilt; break;
+  default:                    ShouldNotReachHere();
   }
 
   // Start filtering what we add to the remembered set. If the object is
@@ -270,7 +243,7 @@ void HeapRegionDCTOC::walk_mem_region_with_cl(MemRegion mr,
       break;
 
     case IntoCSFilterKind: {
-      FilterIntoCSClosure filt(this, g1h, cl, rp);
+      FilterIntoCSClosure filt(this, g1h, cl);
       bottom = walk_mem_region_loop(&filt, g1h, _hr, bottom, top);
       break;
     }