8351921: G1: Pinned regions with pinned objects only reachable by native code crash VM

Reviewed-by: ayang, iwalulya

Author: Thomas Schatzl

src/hotspot/share/gc/g1/g1Policy.cpp

@@ -660,13 +660,6 @@ void G1Policy::record_concurrent_refinement_stats(size_t pending_cards,
 
 bool G1Policy::should_retain_evac_failed_region(uint index) const {
   size_t live_bytes = _g1h->region_at(index)->live_bytes();
-
-#ifdef ASSERT
-  G1HeapRegion* r = _g1h->region_at(index);
-  assert(live_bytes != 0,
-         "live bytes not set for %u used %zu garbage %zu cm-live %zu pinned %d",
-         index, r->used(), r->garbage_bytes(), live_bytes, r->has_pinned_objects());
-#endif
   size_t threshold = G1RetainRegionLiveThresholdPercent * G1HeapRegion::GrainBytes / 100;
   return live_bytes < threshold;
 }

src/hotspot/share/gc/g1/g1YoungCollector.cpp

@@ -287,6 +287,8 @@ class G1PrepareEvacuationTask : public WorkerTask {
   class G1PrepareRegionsClosure : public G1HeapRegionClosure {
     G1CollectedHeap* _g1h;
     G1PrepareEvacuationTask* _parent_task;
+    G1EvacFailureRegions* _evac_failure_regions;
+    uint _worker_id;
     uint _worker_humongous_total;
     uint _worker_humongous_candidates;
 
@@ -361,9 +363,14 @@ class G1PrepareEvacuationTask : public WorkerTask {
     }
 
   public:
-    G1PrepareRegionsClosure(G1CollectedHeap* g1h, G1PrepareEvacuationTask* parent_task) :
+    G1PrepareRegionsClosure(G1CollectedHeap* g1h,
+                            G1PrepareEvacuationTask* parent_task,
+                            G1EvacFailureRegions* evac_failure_regions,
+                            uint worker_id) :
       _g1h(g1h),
       _parent_task(parent_task),
+      _evac_failure_regions(evac_failure_regions),
+      _worker_id(worker_id),
       _worker_humongous_total(0),
       _worker_humongous_candidates(0) { }
 
@@ -373,6 +380,13 @@ class G1PrepareEvacuationTask : public WorkerTask {
     }
 
     virtual bool do_heap_region(G1HeapRegion* hr) {
+      // All pinned regions in the collection set must be registered as failed
+      // regions here as there is no guarantee that there is a reference
+      // reachable by Java code (i.e. only by native code).
+      if (hr->in_collection_set() && hr->has_pinned_objects()) {
+        _evac_failure_regions->record(_worker_id, hr->hrm_index(), true /* cause_pinned */);
+      }
+
       // First prepare the region for scanning
       _g1h->rem_set()->prepare_region_for_scan(hr);
 
@@ -415,22 +429,25 @@ class G1PrepareEvacuationTask : public WorkerTask {
   };
 
   G1CollectedHeap* _g1h;
+  G1EvacFailureRegions* _evac_failure_regions;
   G1HeapRegionClaimer _claimer;
   volatile uint _humongous_total;
   volatile uint _humongous_candidates;
 
   G1MonotonicArenaMemoryStats _all_card_set_stats;
 
 public:
-  G1PrepareEvacuationTask(G1CollectedHeap* g1h) :
+  G1PrepareEvacuationTask(G1CollectedHeap* g1h, G1EvacFailureRegions* evac_failure_regions) :
     WorkerTask("Prepare Evacuation"),
     _g1h(g1h),
+    _evac_failure_regions(evac_failure_regions),
     _claimer(_g1h->workers()->active_workers()),
     _humongous_total(0),
-    _humongous_candidates(0) { }
+    _humongous_candidates(0),
+    _all_card_set_stats() { }
 
   void work(uint worker_id) {
-    G1PrepareRegionsClosure cl(_g1h, this);
+    G1PrepareRegionsClosure cl(_g1h, this, _evac_failure_regions, worker_id);
     _g1h->heap_region_par_iterate_from_worker_offset(&cl, &_claimer, worker_id);
 
     MutexLocker x(G1RareEvent_lock, Mutex::_no_safepoint_check_flag);
@@ -472,7 +489,8 @@ void G1YoungCollector::set_young_collection_default_active_worker_threads(){
   log_info(gc,task)("Using %u workers of %u for evacuation", active_workers, workers()->max_workers());
 }
 
-void G1YoungCollector::pre_evacuate_collection_set(G1EvacInfo* evacuation_info) {
+void G1YoungCollector::pre_evacuate_collection_set(G1EvacInfo* evacuation_info,
+                                                   G1EvacFailureRegions* evac_failure_regions) {
   // Flush various data in thread-local buffers to be able to determine the collection
   // set
   {
@@ -511,7 +529,7 @@ void G1YoungCollector::pre_evacuate_collection_set(G1EvacInfo* evacuation_info)
   }
 
   {
-    G1PrepareEvacuationTask g1_prep_task(_g1h);
+    G1PrepareEvacuationTask g1_prep_task(_g1h, evac_failure_regions);
     Tickspan task_time = run_task_timed(&g1_prep_task);
 
     G1MonotonicArenaMemoryStats sampled_card_set_stats = g1_prep_task.all_card_set_stats();
@@ -1104,7 +1122,7 @@ void G1YoungCollector::collect() {
     // other trivial setup above).
     policy()->record_young_collection_start();
 
-    pre_evacuate_collection_set(jtm.evacuation_info());
+    pre_evacuate_collection_set(jtm.evacuation_info(), &_evac_failure_regions);
 
     G1ParScanThreadStateSet per_thread_states(_g1h,
                                               workers()->active_workers(),

src/hotspot/share/gc/g1/g1YoungCollector.hpp

@@ -95,7 +95,8 @@ class G1YoungCollector {
 
   void set_young_collection_default_active_worker_threads();
 
-  void pre_evacuate_collection_set(G1EvacInfo* evacuation_info);
+  void pre_evacuate_collection_set(G1EvacInfo* evacuation_info,
+                                   G1EvacFailureRegions* evac_failure_regions);
   // Actually do the work of evacuating the parts of the collection set.
   // The has_optional_evacuation_work flag for the initial collection set
   // evacuation indicates whether one or more optional evacuation steps may

test/hotspot/jtreg/gc/g1/pinnedobjs/TestPinnedEvacEmpty.java

@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2025, 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
+ * @summary Test that pinned regions with no Java references into them
+ *          do not make the garbage collector reclaim that region.
+ *          This test simulates this behavior using Whitebox/Unsafe methods
+ *          to pin a Java object in a region with no other pinnable objects and
+*           lose the reference to it before the garbage collection.
+ * @requires vm.gc.G1
+ * @requires vm.debug
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc:+open
+ *          java.management
+ * @build jdk.test.whitebox.WhiteBox
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI -XX:+UseG1GC
+ *      -Xbootclasspath/a:. -Xlog:gc=debug,gc+ergo+cset=trace,gc+phases=debug -XX:G1HeapRegionSize=1m -Xms30m  gc.g1.pinnedobjs.TestPinnedEvacEmpty
+ */
+
+package gc.g1.pinnedobjs;
+
+import jdk.internal.misc.Unsafe;
+
+import jdk.test.lib.Asserts;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.whitebox.WhiteBox;
+
+public class TestPinnedEvacEmpty {
+
+    private static final jdk.internal.misc.Unsafe unsafe = Unsafe.getUnsafe();
+    private static final WhiteBox wb = WhiteBox.getWhiteBox();
+
+    private static final long objSize = wb.getObjectSize(new Object());
+
+    // How many j.l.Object should we allocate when creating garbage.
+    private static final long numAllocations = 1024 * 1024 * 3 / objSize;
+
+    public static void main(String[] args) throws Exception {
+        // Remove garbage from VM initialization.
+        wb.fullGC();
+
+        // Allocate garbage so that the target object will be in a new region.
+        for (int i = 0; i < numAllocations; i++) {
+          Object z = new Object();
+        }
+        int[] o = new int[100];  // The target object to pin.
+        // Further allocate garbage so that any additional allocations of potentially
+        // pinned objects can not be allocated in the same region as the target object.
+        for (int i = 0; i < numAllocations; i++) {
+          Object z = new Object();
+        }
+
+        Asserts.assertTrue(!wb.isObjectInOldGen(o), "should not be in old gen already");
+
+        // Pin the object.
+        wb.pinObject(o);
+
+        // And forget the (Java) reference to the int array. After this, the garbage
+        // collection should find a completely empty pinned region. The collector
+        // must not collect/free it.
+        o = null;
+
+        // Do garbage collection to zap the data in the pinned region.
+        wb.youngGC();
+    }
+}
+