src/share/vm/gc/g1/heapRegionRemSet.cpp

rev 9982 : 8147087: Race when reusing PerRegionTable bitmaps may result in dropped remembered set entries
Summary: Do not make reused PRTs available to other threads before the bitmap of the PRT has been cleared.
Contributed-by: Poonam Bajaj <poonam.bajaj@oracle.com>, Thomas Schatzl <thomas.schatzl@oracle.com>
rev 9983 : [mq]: 8147087-comments

@@ -1,7 +1,7 @@
 /*
- * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License version 2 only, as
  * published by the Free Software Foundation.

@@ -108,11 +108,13 @@
     }
   }
 
 public:
 
-  HeapRegion* hr() const { return _hr; }
+  HeapRegion* hr() const { 
+    return (HeapRegion*) OrderAccess::load_ptr_acquire(&_hr);
+  }
 
   jint occupied() const {
     // Overkill, but if we ever need it...
     // guarantee(_occupied == _bm.count_one_bits(), "Check");
     return _occupied;

@@ -121,14 +123,16 @@
   void init(HeapRegion* hr, bool clear_links_to_all_list) {
     if (clear_links_to_all_list) {
       set_next(NULL);
       set_prev(NULL);
     }
-    _hr = hr;
     _collision_list_next = NULL;
     _occupied = 0;
     _bm.clear();
+    // Make sure that the bitmap clearing above has been finished before publishing
+    // this PRT to concurrent threads.
+    OrderAccess::release_store_ptr(&_hr, hr);
   }
 
   void add_reference(OopOrNarrowOopStar from) {
     add_reference_work(from, /*parallel*/ true);
   }

@@ -355,21 +359,21 @@
   uint cur_hrm_ind = _hr->hrm_index();
 
   int from_card = (int)(uintptr_t(from) >> CardTableModRefBS::card_shift);
 
   if (G1FromCardCache::contains_or_replace(tid, cur_hrm_ind, from_card)) {
-    assert(contains_reference(from), "We just added it!");
+    assert(contains_reference(from), "We just found " PTR_FORMAT " in the FromCardCache", p2i(from));
     return;
   }
 
   // Note that this may be a continued H region.
   HeapRegion* from_hr = _g1h->heap_region_containing(from);
   RegionIdx_t from_hrm_ind = (RegionIdx_t) from_hr->hrm_index();
 
   // If the region is already coarsened, return.
   if (_coarse_map.at(from_hrm_ind)) {
-    assert(contains_reference(from), "We just added it!");
+    assert(contains_reference(from), "We just found " PTR_FORMAT " in the Coarse table", p2i(from));
     return;
   }
 
   // Otherwise find a per-region table to add it to.
   size_t ind = from_hrm_ind & _mod_max_fine_entries_mask;

@@ -386,11 +390,11 @@
       CardIdx_t card_index = from_card - from_hr_bot_card_index;
       assert(0 <= card_index && (size_t)card_index < HeapRegion::CardsPerRegion,
              "Must be in range.");
       if (G1HRRSUseSparseTable &&
           _sparse_table.add_card(from_hrm_ind, card_index)) {
-        assert(contains_reference_locked(from), "We just added it!");
+        assert(contains_reference_locked(from), "We just added " PTR_FORMAT " to the Sparse table", p2i(from));
         return;
       }
 
       if (_n_fine_entries == _max_fine_entries) {
         prt = delete_region_table();

@@ -436,11 +440,11 @@
   // possibility of concurrent reuse.  But see head comment of
   // OtherRegionsTable for why this is OK.
   assert(prt != NULL, "Inv");
 
   prt->add_reference(from);
-  assert(contains_reference(from), "We just added it!");
+  assert(contains_reference(from), "We just added " PTR_FORMAT " to the PRT", p2i(from));
 }
 
 PerRegionTable*
 OtherRegionsTable::find_region_table(size_t ind, HeapRegion* hr) const {
   assert(ind < _max_fine_entries, "Preconditions.");