From d0fd898a96eb0bb8b463d3f44e300e58d0ec46ee Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Mon, 26 Jan 2026 09:19:14 +0000 Subject: [PATCH] 8375314: Parallel: Crash iterating over unloaded classes for ObjectCountAfterGC event Reviewed-by: iwalulya Backport-of: f3381f0ffe2207e1765558f6f49e5a0280a3f920 --- src/hotspot/share/gc/g1/g1CollectedHeap.hpp | 1 - .../share/gc/g1/g1CollectedHeap.inline.hpp | 8 +- .../share/gc/parallel/psParallelCompact.cpp | 23 ++-- .../share/gc/parallel/psParallelCompact.hpp | 1 + src/hotspot/share/gc/shared/collectedHeap.hpp | 4 +- .../share/gc/shared/collectedHeap.inline.hpp | 9 +- .../gc/parallel/TestObjectCountAfterGC.java | 104 ++++++++++++++++++ 7 files changed, 134 insertions(+), 16 deletions(-) create mode 100644 test/hotspot/jtreg/gc/parallel/TestObjectCountAfterGC.java diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp index aff7166d391..83e9046735d 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.hpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.hpp @@ -1267,7 +1267,6 @@ public: bool is_marked(oop obj) const; - inline static bool is_obj_filler(const oop obj); // Determine if an object is dead, given the object and also // the region to which the object belongs. inline bool is_obj_dead(const oop obj, const G1HeapRegion* hr) const; diff --git a/src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp b/src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp index abd61e72d57..04df583def6 100644 --- a/src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp +++ b/src/hotspot/share/gc/g1/g1CollectedHeap.inline.hpp @@ -38,6 +38,7 @@ #include "gc/g1/g1Policy.hpp" #include "gc/g1/g1RegionPinCache.inline.hpp" #include "gc/g1/g1RemSet.hpp" +#include "gc/shared/collectedHeap.inline.hpp" #include "gc/shared/markBitMap.inline.hpp" #include "gc/shared/taskqueue.inline.hpp" #include "oops/stackChunkOop.hpp" @@ -230,16 +231,11 @@ inline bool G1CollectedHeap::requires_barriers(stackChunkOop obj) const { return !heap_region_containing(obj)->is_young(); // is_in_young does an unnecessary null check } -inline bool G1CollectedHeap::is_obj_filler(const oop obj) { - Klass* k = obj->klass_without_asserts(); - return k == Universe::fillerArrayKlass() || k == vmClasses::FillerObject_klass(); -} - inline bool G1CollectedHeap::is_obj_dead(const oop obj, const G1HeapRegion* hr) const { assert(!hr->is_free(), "looking up obj " PTR_FORMAT " in Free region %u", p2i(obj), hr->hrm_index()); if (hr->is_in_parsable_area(obj)) { // This object is in the parsable part of the heap, live unless scrubbed. - return is_obj_filler(obj); + return is_filler_object(obj); } else { // From Remark until a region has been concurrently scrubbed, parts of the // region is not guaranteed to be parsable. Use the bitmap for liveness. diff --git a/src/hotspot/share/gc/parallel/psParallelCompact.cpp b/src/hotspot/share/gc/parallel/psParallelCompact.cpp index b1b07b4bc5c..bab72296d4c 100644 --- a/src/hotspot/share/gc/parallel/psParallelCompact.cpp +++ b/src/hotspot/share/gc/parallel/psParallelCompact.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2026, 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 @@ -44,6 +44,7 @@ #include "gc/parallel/psStringDedup.hpp" #include "gc/parallel/psYoungGen.hpp" #include "gc/shared/classUnloadingContext.hpp" +#include "gc/shared/collectedHeap.inline.hpp" #include "gc/shared/fullGCForwarding.inline.hpp" #include "gc/shared/gcCause.hpp" #include "gc/shared/gcHeapSummary.hpp" @@ -932,6 +933,17 @@ void PSParallelCompact::summary_phase(bool should_do_max_compaction) } } +void PSParallelCompact::report_object_count_after_gc() { + GCTraceTime(Debug, gc, phases) tm("Report Object Count", &_gc_timer); + // The heap is compacted, all objects are iterable. However there may be + // filler objects in the heap which we should ignore. + class SkipFillerObjectClosure : public BoolObjectClosure { + public: + bool do_object_b(oop obj) override { return !CollectedHeap::is_filler_object(obj); } + } cl; + _gc_tracer.report_object_count_after_gc(&cl, &ParallelScavengeHeap::heap()->workers()); +} + bool PSParallelCompact::invoke(bool clear_all_soft_refs, bool should_do_max_compaction) { assert(SafepointSynchronize::is_at_safepoint(), "should be at safepoint"); assert(Thread::current() == (Thread*)VMThread::vm_thread(), @@ -1027,6 +1039,8 @@ bool PSParallelCompact::invoke(bool clear_all_soft_refs, bool should_do_max_comp heap->print_heap_change(pre_gc_values); + report_object_count_after_gc(); + // Track memory usage and detect low memory MemoryService::track_memory_usage(); heap->update_counters(); @@ -1274,10 +1288,6 @@ void PSParallelCompact::marking_phase(ParallelOldTracer *gc_tracer) { } } - { - GCTraceTime(Debug, gc, phases) tm("Report Object Count", &_gc_timer); - _gc_tracer.report_object_count_after_gc(is_alive_closure(), &ParallelScavengeHeap::heap()->workers()); - } #if TASKQUEUE_STATS ParCompactionManager::print_and_reset_taskqueue_stats(); #endif @@ -1835,8 +1845,7 @@ void PSParallelCompact::verify_filler_in_dense_prefix() { oop obj = cast_to_oop(cur_addr); oopDesc::verify(obj); if (!mark_bitmap()->is_marked(cur_addr)) { - Klass* k = cast_to_oop(cur_addr)->klass(); - assert(k == Universe::fillerArrayKlass() || k == vmClasses::FillerObject_klass(), "inv"); + assert(CollectedHeap::is_filler_object(cast_to_oop(cur_addr)), "inv"); } cur_addr += obj->size(); } diff --git a/src/hotspot/share/gc/parallel/psParallelCompact.hpp b/src/hotspot/share/gc/parallel/psParallelCompact.hpp index 2297d720b35..4ac9395d727 100644 --- a/src/hotspot/share/gc/parallel/psParallelCompact.hpp +++ b/src/hotspot/share/gc/parallel/psParallelCompact.hpp @@ -749,6 +749,7 @@ private: // Move objects to new locations. static void compact(); + static void report_object_count_after_gc(); // Add available regions to the stack and draining tasks to the task queue. static void prepare_region_draining_tasks(uint parallel_gc_threads); diff --git a/src/hotspot/share/gc/shared/collectedHeap.hpp b/src/hotspot/share/gc/shared/collectedHeap.hpp index 6f335b1cdf4..363ccf321b2 100644 --- a/src/hotspot/share/gc/shared/collectedHeap.hpp +++ b/src/hotspot/share/gc/shared/collectedHeap.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2026, 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 @@ -309,6 +309,8 @@ protected: fill_with_object(start, pointer_delta(end, start), zap); } + inline static bool is_filler_object(oop obj); + virtual void fill_with_dummy_object(HeapWord* start, HeapWord* end, bool zap); static size_t min_dummy_object_size() { return oopDesc::header_size(); diff --git a/src/hotspot/share/gc/shared/collectedHeap.inline.hpp b/src/hotspot/share/gc/shared/collectedHeap.inline.hpp index 3900d1809ee..194c1fe0bf2 100644 --- a/src/hotspot/share/gc/shared/collectedHeap.inline.hpp +++ b/src/hotspot/share/gc/shared/collectedHeap.inline.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2001, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2001, 2026, 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 @@ -27,7 +27,9 @@ #include "gc/shared/collectedHeap.hpp" +#include "classfile/vmClasses.hpp" #include "gc/shared/memAllocator.hpp" +#include "memory/universe.hpp" #include "oops/oop.inline.hpp" #include "utilities/align.hpp" @@ -50,4 +52,9 @@ inline void CollectedHeap::add_vmthread_cpu_time(jlong time) { _vmthread_cpu_time += time; } +inline bool CollectedHeap::is_filler_object(oop obj) { + Klass* k = obj->klass_without_asserts(); + return k == Universe::fillerArrayKlass() || k == vmClasses::FillerObject_klass(); +} + #endif // SHARE_GC_SHARED_COLLECTEDHEAP_INLINE_HPP diff --git a/test/hotspot/jtreg/gc/parallel/TestObjectCountAfterGC.java b/test/hotspot/jtreg/gc/parallel/TestObjectCountAfterGC.java new file mode 100644 index 00000000000..e61e7518938 --- /dev/null +++ b/test/hotspot/jtreg/gc/parallel/TestObjectCountAfterGC.java @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2026, 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. + */ + +package gc.parallel; + +/* + * @test TestObjectCountAfterGC + * @bug 8375314 + * @summary Verifies that the HeapInspection VM operation for the ObjectCountAfterGC JFR event does not crash the VM. + * Creates a set of custom classes that are about to be unloaded to cause metaspace to uncommit pages. When + * the execution of the heap inspection iterates over the heap, it will come across these unloaded classes + * referencing uncommitted memory, crashing. + * @requires vm.gc.Parallel + * @requires vm.opt.final.ClassUnloading + * @library /test/lib + * @library /testlibrary/asm + * @modules java.base/jdk.internal.misc + * @run main/othervm -XX:+UseParallelGC -Xlog:gc=debug,metaspace=info -XX:StartFlightRecording:gc=all,duration=1s,filename=myrecording.jfr + * gc.parallel.TestObjectCountAfterGC + */ + +import java.lang.ref.Reference; + +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.Opcodes; + +public class TestObjectCountAfterGC { + + static final String className = "ClassToLoadUnload"; + + public static void main(String args[]) throws Exception { + final int KEEPALIVE_LENGTH = 100; + + Object[] keepalive = new Object[KEEPALIVE_LENGTH]; + + for (int i = 1; i < 1000; i++) { + ClassLoader cl = new MyClassLoader(); + Object o = null; + // Create some random kept alive objects so that the + // compaction regions are not totally empty and the + // heap inspection VM operation needs to iterate them. + keepalive[(i / KEEPALIVE_LENGTH) % KEEPALIVE_LENGTH] = new int[100]; + o = cl.loadClass(className + i).newInstance(); + + cl = null; + o = null; + } + + // There is heap inspection VM operation for the ObjectCountAfterGC event + // when JFR stops recording. + + Reference.reachabilityFence(keepalive); + } +} + +class MyClassLoader extends ClassLoader { + + // Create a class of the given name with a default constructor. + public byte[] createClass(String name) { + ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES); + cw.visit(Opcodes.V1_8, Opcodes.ACC_PUBLIC | Opcodes.ACC_SUPER, name, null, "java/lang/Object", null); + // Add default constructor that just calls the super class constructor. + MethodVisitor mv = cw.visitMethod(Opcodes.ACC_PUBLIC, "", "()V", null, null); + mv.visitCode(); + mv.visitVarInsn(Opcodes.ALOAD, 0); + mv.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "", "()V", false); + mv.visitInsn(Opcodes.RETURN); + mv.visitMaxs(0, 0); + mv.visitEnd(); + return cw.toByteArray(); + } + + // If the given name starts with "TestObjectCountAfterGC" create a new class on the fly, + // delegate otherwise. + public Class loadClass(String name) throws ClassNotFoundException { + if (!name.startsWith(TestObjectCountAfterGC.className)) { + return super.loadClass(name); + } + byte[] cls = createClass(name); + return defineClass(name, cls, 0, cls.length, null); + } + } +