8139424: SIGSEGV, Problematic frame: # V [libjvm.so+0xd0c0cc] void...

8139424: SIGSEGV, Problematic frame: # V [libjvm.so+0xd0c0cc] void InstanceKlass::oop_oop_iterate_oop_maps_specialized<true,oopDesc*,MarkAndPushClosure>
Summary: The crash was caused by a faulty eager humongous reclaim. The reason for reclaiming a live object was that the call to cleanupHRRS was done after dirtying cards and clearing the remembered sets for the humongous object. This could lead to one or many cards being missed.
Reviewed-by: tbenson, kbarrett, tschatzl

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

@@ -3767,6 +3767,9 @@ class RegisterHumongousWithInCSetFastTestClosure : public HeapRegionClosure {
             _dcq.enqueue(card_ptr);
           }
         }
+        assert(hrrs.n_yielded() == r->rem_set()->occupied(),
+               err_msg("Remembered set hash maps out of sync, cur: " SIZE_FORMAT " entries, next: " SIZE_FORMAT " entries",
+               hrrs.n_yielded(), r->rem_set()->occupied()));
         r->rem_set()->clear_locked();
       }
       assert(r->rem_set()->is_empty(), "At this point any humongous candidate remembered set must be empty.");
@@ -4089,6 +4092,13 @@ G1CollectedHeap::do_collection_pause_at_safepoint(double target_pause_time_ms) {
 
         g1_policy()->finalize_cset(target_pause_time_ms, evacuation_info);
 
+        // Make sure the remembered sets are up to date. This needs to be
+        // done before register_humongous_regions_with_cset(), because the
+        // remembered sets are used there to choose eager reclaim candidates.
+        // If the remembered sets are not up to date we might miss some
+        // entries that need to be handled.
+        g1_rem_set()->cleanupHRRS();
+
         register_humongous_regions_with_in_cset_fast_test();
 
         assert(check_cset_fast_test(), "Inconsistency in the InCSetState table.");

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

@@ -335,7 +335,6 @@ void G1RemSet::oops_into_collection_set_do(G1ParPushHeapRSClosure* oc,
 }
 
 void G1RemSet::prepare_for_oops_into_collection_set_do() {
-  cleanupHRRS();
   _g1->set_refine_cte_cl_concurrency(false);
   DirtyCardQueueSet& dcqs = JavaThread::dirty_card_queue_set();
   dcqs.concatenate_logs();

test/gc_implementation/g1/TestNoEagerReclaimOfHumongousRegions.java

+/*
+ * Copyright (c) 2015, 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.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test TestNoEagerReclaimOfHumongousRegions
+ * @bug 8139424
+ * @summary Test to check that a live humongous object is not eagerly reclaimed. This is a regression test for
+ *          8139424 and the test will crash if an eager reclaim occur. The test is not 100% deterministic and
+ *          might pass even if there are problems in the code, but it will never crash unless there is a problem.
+ * @requires vm.gc=="G1" | vm.gc=="null"
+ * @key gc
+ * @library /testlibrary /testlibrary/whitebox
+ * @modules java.base/sun.misc
+ * @build TestNoEagerReclaimOfHumongousRegions
+ * @run main ClassFileInstaller sun.hotspot.WhiteBox
+ *                              sun.hotspot.WhiteBox$WhiteBoxPermission
+ * @run main/othervm -Xbootclasspath/a:. -XX:+PrintGC -XX:+UseG1GC -XX:MaxTenuringThreshold=0 -XX:G1RSetSparseRegionEntries=32 -XX:G1HeapRegionSize=1m -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -XX:+UnlockExperimentalVMOptions -XX:+G1TraceEagerReclaimHumongousObjects TestNoEagerReclaimOfHumongousRegions
+ */
+
+import java.util.LinkedList;
+
+import sun.hotspot.WhiteBox;
+
+public class TestNoEagerReclaimOfHumongousRegions {
+    // Helper class to keep reference to humongous byte[].
+    static class LargeRef {
+        private byte[] _ref;
+
+        LargeRef(byte[] ref) {
+            _ref = ref;
+        }
+
+        byte[] ref() { return _ref; }
+    }
+
+    static LargeRef humongous_reference_holder;
+
+    public static void main(String[] args) throws InterruptedException{
+        WhiteBox wb = WhiteBox.getWhiteBox();
+        LinkedList<Object> garbageAndRefList = new LinkedList<Object>();
+        // Creating a 1M large byte array. Since the test specifies the heap
+        // region size to be 1m this will be a humongous object. We then
+        // store a pointer to the array in the static object to keep it live
+        // during the whole test.
+        humongous_reference_holder = new LargeRef(new byte[1 * 1024 * 1024]);
+
+        // Create some garbage and a reference to the humongous object each round.
+        for (int i = 0; i < 32; i++) {
+            garbageAndRefList.add(new byte[400*1000]);
+            garbageAndRefList.add(new LargeRef(humongous_reference_holder.ref()));
+
+            // Promote to old, goal is to get rem-set entries for the humongous
+            // object from different regions. The test specifies MaxTenuringThreshold=0,
+            // this will make sure we get objects promoted to old at once.
+            wb.youngGC();
+        }
+        // Clear the garbage and reference list.
+        garbageAndRefList.clear();
+
+        // Run a concurrent mark cycle to mark all references but the static one as dead.
+        wb.g1StartConcMarkCycle();
+        while (wb.g1InConcurrentMark()) {
+            Thread.sleep(100);
+        }
+
+        // Run a young collection to make sure humongous object still can't be eagerly reclaimed.
+        wb.youngGC();
+        // Will crash/assert if humongous object has been reclaimed.
+        wb.fullGC();
+    }
+}