From cce810ed3086ae8bc2f6b94aa83ab2afe1374efc Mon Sep 17 00:00:00 2001 From: Xueming Shen Date: Fri, 5 Jun 2026 17:44:55 +0000 Subject: [PATCH 1/3] 8385355: NullPointerException in jdk.tools.jlink.internal.ImageResourcesTree after JDK-8377070 Reviewed-by: alanb Backport-of: 92298786486daf092c8eb8f278818441df1f6b51 --- .../jlink/internal/ImageResourcesTree.java | 40 +++++++++++++++---- .../jdk/internal/jimage/ImageReaderTest.java | 32 ++++++++++++++- 2 files changed, 62 insertions(+), 10 deletions(-) diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java index 2f8143820ca7..ab0809348cab 100644 --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageResourcesTree.java @@ -235,22 +235,46 @@ private void processPath( return; } String modName = fullPath.substring(1, modEnd); - String pkgPath = fullPath.substring(modEnd + 1); + String resPath = fullPath.substring(modEnd + 1); + int pathEnd = resPath.lastIndexOf('/'); Node parentNode = getDirectoryNode(modName, modulesRoot); boolean isPreviewPath = false; - if (pkgPath.startsWith(PREVIEW_PREFIX)) { + if (resPath.startsWith("META-INF/")) { + parentNode = getDirectoryNode("META-INF", parentNode); + if (!resPath.startsWith(PREVIEW_PREFIX)) { + // Non-preview META-INF paths are resources, not package + // hierarchies, so directory and file names may contain dots. + for (int i = "META-INF".length(), j; i != pathEnd; i = j) { + j = resPath.indexOf('/', i + 1); + parentNode = getDirectoryNode(resPath.substring(i + 1, j), parentNode); + } + String resourceName = resPath.substring(pathEnd + 1); + Node resourceNode = parentNode.getChildren(resourceName); + if (resourceNode == null) { + new ResourceNode(resourceName, parentNode); + } else if (!(resourceNode instanceof ResourceNode)) { + throw new InvalidTreeException(resourceNode); + } + return; + } // For preview paths, process nodes relative to the preview directory. - pkgPath = pkgPath.substring(PREVIEW_PREFIX.length()); - Node metaInf = getDirectoryNode("META-INF", parentNode); - parentNode = getDirectoryNode("preview", metaInf); + parentNode = getDirectoryNode("preview", parentNode); + resPath = resPath.substring(PREVIEW_PREFIX.length()); + pathEnd -= PREVIEW_PREFIX.length(); isPreviewPath = true; } - int pathEnd = pkgPath.lastIndexOf('/'); // From invariants tested above, this must now be well-formed. - String fullPkgName = (pathEnd == -1) ? "" : pkgPath.substring(0, pathEnd).replace('/', '.'); - String resourceName = pkgPath.substring(pathEnd + 1); + String pkgPath = (pathEnd == -1) ? "" : resPath.substring(0, pathEnd); + if (pkgPath.contains(".")) { + // Non META-INF entries are package paths. Dots in path segment + // names would otherwise be confused with package separators. + System.err.println("Invalid package path, skipping " + pkgPath); + return; + } + String fullPkgName = pkgPath.replace('/', '.'); + String resourceName = resPath.substring(pathEnd + 1); // Intermediate packages are marked "empty" (no resources). This might // later be merged with a non-empty link for the same package. ModuleLink emptyLink = ModuleLink.forEmptyPackage(modName, isPreviewPath); diff --git a/test/jdk/jdk/internal/jimage/ImageReaderTest.java b/test/jdk/jdk/internal/jimage/ImageReaderTest.java index b8f7763e97e8..5104bb97f956 100644 --- a/test/jdk/jdk/internal/jimage/ImageReaderTest.java +++ b/test/jdk/jdk/internal/jimage/ImageReaderTest.java @@ -37,6 +37,7 @@ import tests.JImageGenerator; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.Arrays; import java.util.List; @@ -58,6 +59,7 @@ /* * @test + * @bug 8385355 * @summary Tests for ImageReader. * @modules java.base/jdk.internal.jimage * jdk.jlink/jdk.tools.jlink.internal @@ -72,13 +74,18 @@ /// There is no mutable test instance state to worry about. @TestInstance(PER_CLASS) public class ImageReaderTest { - // The '@' prefix marks the entry as a preview entry which will be placed in - // the '/modules//META-INF/preview/...' namespace. + // The '@' prefix marks the entry as a preview class entry which will be placed in + // the '/modules//META-INF/preview/...' namespace. The '!' prefix marks + // the entry as a non-class resource path. private static final Map> IMAGE_ENTRIES = Map.of( "modfoo", Arrays.asList( "com.foo.HasPreviewVersion", "com.foo.NormalFoo", "com.foo.bar.NormalBar", + "!META-INF/maven/com.google.code.findbugs/jsr305/pom.properties", + "!META-INF/z", + "!META-INF/collision/child.properties", + "!META-INF/collision", // Replaces original class in preview mode. "@com.foo.HasPreviewVersion", // New class in existing package in preview mode. @@ -138,6 +145,23 @@ public void testModuleResources() throws IOException { } } + @Test + public void testMetaInfResourcesAreNotPackagePaths() throws IOException { + for (PreviewMode mode : List.of(PreviewMode.ENABLED, PreviewMode.DISABLED)) { + try (ImageReader reader = ImageReader.open(image, mode)) { + assertResource(reader, "modfoo", "META-INF/maven/com.google.code.findbugs/jsr305/pom.properties"); + assertResource(reader, "modfoo", "META-INF/z"); + assertResource(reader, "modfoo", "META-INF/collision/child.properties"); + assertDirContents(reader, "/modules/modfoo/META-INF", "MANIFEST.MF", "collision", "maven", "z"); + assertDirContents(reader, "/modules/modfoo/META-INF/collision", "child.properties"); + assertDirContents(reader, "/modules/modfoo/META-INF/maven", "com.google.code.findbugs"); + assertDirContents(reader, "/modules/modfoo/META-INF/maven/com.google.code.findbugs", "jsr305"); + assertDirContents(reader, "/modules/modfoo/META-INF/maven/com.google.code.findbugs/jsr305", "pom.properties"); + assertAbsent(reader, "/modules/modfoo/META-INF/maven/com/google/code/findbugs/jsr305/pom.properties"); + } + } + } + @ParameterizedTest @CsvSource(delimiter = ':', value = { "modfoo:com/foo/HasPreviewVersion.class", @@ -416,6 +440,10 @@ public static Path buildJImage(Map> entries) { jar.addEntry("module-info.class", InMemoryJavaCompiler.compile("module-info", moduleInfo)); classes.forEach(fqn -> { + if (fqn.startsWith("!")) { + jar.addEntry(fqn.substring(1), "resource".getBytes(StandardCharsets.UTF_8)); + return; + } boolean isPreviewEntry = fqn.startsWith("@"); if (isPreviewEntry) { fqn = fqn.substring(1); From 80ec2e59388dd3d44719ea072f44e4eedee0c050 Mon Sep 17 00:00:00 2001 From: Patricio Chilano Mateo Date: Wed, 10 Jun 2026 14:39:43 +0000 Subject: [PATCH 2/3] 8385806: Assert failed when running Skynet.java with continuation trace logging enabled Reviewed-by: coleenp Backport-of: 21482fa7dba9aa649a37abe640910eda9d505b85 --- src/hotspot/share/runtime/frame.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/hotspot/share/runtime/frame.cpp b/src/hotspot/share/runtime/frame.cpp index a1c33f4fda00..b983b4648d4a 100644 --- a/src/hotspot/share/runtime/frame.cpp +++ b/src/hotspot/share/runtime/frame.cpp @@ -1237,9 +1237,6 @@ void frame::interpreter_frame_verify_monitor(BasicObjectLock* value) const { #ifndef PRODUCT -// Returns true iff the address p is readable and *(intptr_t*)p != errvalue -extern "C" bool dbg_is_safe(const void* p, intptr_t errvalue); - class FrameValuesOopClosure: public OopClosure, public DerivedOopClosure { private: GrowableArray* _oops; @@ -1269,17 +1266,13 @@ class FrameValuesOopClosure: public OopClosure, public DerivedOopClosure { _derived->push(derived_loc); } - bool is_good(oop* p) { - return *p == nullptr || (dbg_is_safe(*p, -1) && dbg_is_safe((*p)->klass_without_asserts(), -1) && oopDesc::is_oop_or_null(*p)); - } void describe(FrameValues& values, int frame_no) { for (int i = 0; i < _oops->length(); i++) { oop* p = _oops->at(i); - values.describe(frame_no, (intptr_t*)p, err_msg("oop%s for #%d", is_good(p) ? "" : " (BAD)", frame_no)); + values.describe(frame_no, (intptr_t*)p, err_msg("oop for #%d", frame_no)); } for (int i = 0; i < _narrow_oops->length(); i++) { narrowOop* p = _narrow_oops->at(i); - // we can't check for bad compressed oops, as decoding them might crash values.describe(frame_no, (intptr_t*)p, err_msg("narrow oop for #%d", frame_no)); } assert(_base->length() == _derived->length(), "should be the same"); From 5d29621bae760340aa19c6a909d32d54fd6f0f7e Mon Sep 17 00:00:00 2001 From: Thomas Schatzl Date: Thu, 11 Jun 2026 08:10:39 +0000 Subject: [PATCH 3/3] 8385369: G1: Concurrent Cleanup For Next Mark accesses uncommitted bitmaps after region uncommit Reviewed-by: iwalulya Backport-of: 5719b671a226ac5cfd116e96d0c9b1c25607b41d --- src/hotspot/share/gc/g1/g1ConcurrentMark.cpp | 15 +- .../share/gc/g1/g1UncommitRegionTask.cpp | 6 +- .../share/gc/g1/g1UncommitRegionTask.hpp | 4 +- src/hotspot/share/gc/g1/g1_globals.hpp | 6 +- ...stUncommitDuringConcurrentBitmapClear.java | 133 ++++++++++++++++++ 5 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 test/hotspot/jtreg/resourcehogs/gc/TestUncommitDuringConcurrentBitmapClear.java diff --git a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp index 83dda2a043bb..4afc7fa8ff12 100644 --- a/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp +++ b/src/hotspot/share/gc/g1/g1ConcurrentMark.cpp @@ -714,6 +714,11 @@ class G1ClearBitMapTask : public WorkerTask { } HeapWord* region_clear_limit(G1HeapRegion* r) { + // A garbage collection might have made the region unavailable after a yield during + // clearing. Just return bottom as the limit, causing the clearing for this region to end. + if (G1CollectedHeap::heap()->region_at_or_null(r->hrm_index()) == nullptr) { + return r->bottom(); + } // During a Concurrent Undo Mark cycle, the per region top_at_mark_start and // live_words data are current wrt to the _mark_bitmap. We use this information // to only clear ranges of the bitmap that require clearing. @@ -743,7 +748,7 @@ class G1ClearBitMapTask : public WorkerTask { } HeapWord* cur = r->bottom(); - HeapWord* const end = region_clear_limit(r); + HeapWord* end = region_clear_limit(r); size_t const chunk_size_in_words = G1ClearBitMapTask::chunk_size() / HeapWordSize; @@ -761,8 +766,12 @@ class G1ClearBitMapTask : public WorkerTask { assert(!suspendible() || _cm->is_in_reset_for_next_cycle(), "invariant"); // Abort iteration if necessary. - if (has_aborted()) { - return true; + if (suspendible() && _cm->do_yield_check()) { + if (_cm->has_aborted()) { + return true; + } + // Re-read end. The region might have been uncommitted. + end = region_clear_limit(r); } } assert(cur >= end, "Must have completed iteration over the bitmap for region %u.", r->hrm_index()); diff --git a/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp b/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp index e1203229557a..f88736c5642f 100644 --- a/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp +++ b/src/hotspot/share/gc/g1/g1UncommitRegionTask.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 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 @@ -58,9 +58,9 @@ void G1UncommitRegionTask::enqueue() { G1UncommitRegionTask* uncommit_task = instance(); if (!uncommit_task->is_active()) { - // Change state to active and schedule using UncommitInitialDelayMs. + // Change state to active and schedule. uncommit_task->set_active(true); - G1CollectedHeap::heap()->service_thread()->schedule_task(uncommit_task, UncommitInitialDelayMs); + G1CollectedHeap::heap()->service_thread()->schedule_task(uncommit_task, G1UncommitInitialDelay); } } diff --git a/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp b/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp index 7c9a25f6857d..835217d2a595 100644 --- a/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp +++ b/src/hotspot/share/gc/g1/g1UncommitRegionTask.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 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 @@ -34,8 +34,6 @@ class G1UncommitRegionTask : public G1ServiceTask { // This limit is small enough to ensure that the duration of each invocation // is short, while still making reasonable progress. static const uint UncommitSizeLimit = 128 * M; - // Initial delay in milliseconds after GC before the regions are uncommitted. - static const uint UncommitInitialDelayMs = 100; // The delay between two uncommit task executions. static const uint UncommitTaskDelayMs = 10; diff --git a/src/hotspot/share/gc/g1/g1_globals.hpp b/src/hotspot/share/gc/g1/g1_globals.hpp index 14daac4800b1..8a346c5c78f3 100644 --- a/src/hotspot/share/gc/g1/g1_globals.hpp +++ b/src/hotspot/share/gc/g1/g1_globals.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 @@ -184,6 +184,10 @@ "shrink attempt.") \ range(0, 100) \ \ + develop(uint, G1UncommitInitialDelay, 100, \ + "Delay in milliseconds until regions just made eligible for " \ + "uncommit are actually uncommitted.") \ + \ product(uint, G1CPUUsageDeviationPercent, 25, DIAGNOSTIC, \ "The acceptable deviation (in percent) from the target GC CPU " \ "usage (based on GCTimeRatio). Creates a tolerance range " \ diff --git a/test/hotspot/jtreg/resourcehogs/gc/TestUncommitDuringConcurrentBitmapClear.java b/test/hotspot/jtreg/resourcehogs/gc/TestUncommitDuringConcurrentBitmapClear.java new file mode 100644 index 000000000000..f79f4992817f --- /dev/null +++ b/test/hotspot/jtreg/resourcehogs/gc/TestUncommitDuringConcurrentBitmapClear.java @@ -0,0 +1,133 @@ +/* + * 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.g1; + +/* + * @test TestUncommitDuringConcurrentBitmapClear + * @bug 8385369 + * @requires vm.gc.G1 + * @requires vm.debug + * @requires vm.flagless + * @requires vm.bits == 64 + * @requires os.maxMemory > 8g + * @summary Verify that G1 does not crash while uncommitting a region whose + * bitmap is currently being cleared. + * Options are geared towards uncommitting aggressively. Also use a large + * region size so that corresponding bitmaps get uncommitted always too. + * @library /test/lib + * @build jdk.test.whitebox.WhiteBox + * @run driver jdk.test.lib.helpers.ClassFileInstaller jdk.test.whitebox.WhiteBox + * @run main/othervm + * -Xbootclasspath/a:. + * -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI + * -Xmx8g + * -Xms32m + * -XX:G1HeapRegionSize=16m + * -XX:+UseG1GC + * -XX:ConcGCThreads=1 + * -XX:GCTimeRatio=1 + * -XX:G1CPUUsageShrinkThreshold=1 + * -XX:G1ShrinkByPercentOfAvailable=100 + * -XX:G1UncommitInitialDelay=0 + * -Xlog:gc+marking,gc,gc+ergo+heap=debug + * gc.g1.TestUncommitDuringConcurrentBitmapClear + */ + +import java.util.concurrent.FutureTask; +import java.util.concurrent.TimeUnit; + +import jdk.test.whitebox.WhiteBox; + +/* + * Repeatedly make the concurrent cycle stop after the cleanup pause, issuing + * young GCs during the Concurrent Cleanup for Next Mark phase. Humongous + * regions allocated and dropped before that should get eager-reclaimed and + * their memory uncommitted while bitmap clearing runs. + */ +public class TestUncommitDuringConcurrentBitmapClear { + + private static final WhiteBox WB = WhiteBox.getWhiteBox(); + + private static final int NumObjs = 400; // Number of humongous objects to allocate + // per attempt. Sized to fill a fair amount of + // the available memory. + private static final int LargeObjSize = 9 * 1024 * 1024; // Large enough to be a humongous object. + + private static Object[] objects; + + private static void test() throws Exception { + + // This task drops the humongous objects, making them eligible for + // uncommit, and starts the concurrent bitmap clearing. While it is + // running, the caller triggers GCs that may or may not trigger the issue. + FutureTask concurrentClearTask = new FutureTask<>(() -> { + objects = null; + WB.concurrentGCRunTo(WB.G1_BEFORE_CLEANUP_COMPLETED); + return null; + }); + + try { + System.out.println("taking control"); + WB.concurrentGCAcquireControl(); + + // Allocate a new set of humongous objects. Acquire control first to avoid + // unnecessary concurrent cycles due to that allocation. We do not need them. + objects = new Object[NumObjs]; + for (int i = 0; i < objects.length; i++) { + objects[i] = new byte[LargeObjSize]; + } + + WB.concurrentGCRunTo(WB.G1_AFTER_CLEANUP_STARTED); + + new Thread(concurrentClearTask).start(); + + int numYoungGCs = 0; + // Execute at least one young GC, even if the concurrent + // clear bitmap finishes very quickly. + do { + WB.youngGC(); + numYoungGCs++; + // Wait a bit. This should give the concurrent clear task a chance + // to finish execution. + Thread.sleep(1); + } while (!concurrentClearTask.isDone() && numYoungGCs < 200); + + concurrentClearTask.get(30, TimeUnit.SECONDS); // Propagates exceptions, if any. + } finally { + WB.concurrentGCRunToIdle(); + + System.out.println("Releasing control"); + WB.concurrentGCReleaseControl(); + } + } + + public static void main(String[] args) throws Exception { + if (!WB.supportsConcurrentGCBreakpoints()) { + throw new RuntimeException("G1 should support GC breakpoints"); + } + for (int i = 0; i < 20; i++) { + test(); + } + } +}