From 6fb20c8a4dd7b7ae187254b87ba3b8fb13deff43 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Thu, 12 Jun 2025 18:03:46 +0200 Subject: [PATCH 01/11] Reapply diffs. --- .../jdk/internal/jimage/ImageReader.java | 769 ++++++++---------- .../jdk/internal/jrtfs/ExplodedImage.java | 4 +- .../jdk/internal/jrtfs/JrtFileSystem.java | 26 +- .../internal/module/SystemModuleFinders.java | 14 +- .../jdk/internal/jimage/JImageReadTest.java | 36 +- 5 files changed, 369 insertions(+), 480 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java b/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java index f12c39f3e814f..5c22a05c1d386 100644 --- a/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java +++ b/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java @@ -25,7 +25,6 @@ package jdk.internal.jimage; import java.io.IOException; -import java.io.InputStream; import java.io.UncheckedIOException; import java.nio.ByteBuffer; import java.nio.ByteOrder; @@ -35,6 +34,7 @@ import java.nio.file.attribute.FileTime; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -42,7 +42,9 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; +import java.util.stream.Stream; /** * @implNote This class needs to maintain JDK 8 source compatibility. @@ -92,57 +94,32 @@ private void requireOpen() { } } + // TODO: Called by SystemImage (now completely pointless). // directory management interface public Directory getRootDirectory() throws IOException { ensureOpen(); - return reader.getRootDirectory(); + return (Directory) findNode("/"); } - + // TODO: Called by SystemImage and SystemModuleFinders. public Node findNode(String name) throws IOException { ensureOpen(); return reader.findNode(name); } + // TODO: Called by SystemImage for JrtFileSystem. public byte[] getResource(Node node) throws IOException { ensureOpen(); return reader.getResource(node); } - public byte[] getResource(Resource rs) throws IOException { - ensureOpen(); - return reader.getResource(rs); - } - - public ImageHeader getHeader() { - requireOpen(); - return reader.getHeader(); - } - + // TODO: Called once from SystemModuleReader::release(). public static void releaseByteBuffer(ByteBuffer buffer) { BasicImageReader.releaseByteBuffer(buffer); } - public String getName() { - requireOpen(); - return reader.getName(); - } - - public ByteOrder getByteOrder() { - requireOpen(); - return reader.getByteOrder(); - } - - public Path getImagePath() { - requireOpen(); - return reader.getImagePath(); - } - - public ImageStringsReader getStrings() { - requireOpen(); - return reader.getStrings(); - } - + // TODO: Avoid leaking ImageLocation into callers (only 3) so we can + // encapsulate better for exploded image etc. public ImageLocation findLocation(String mn, String rn) { requireOpen(); return reader.findLocation(mn, rn); @@ -153,82 +130,69 @@ public boolean verifyLocation(String mn, String rn) { return reader.verifyLocation(mn, rn); } - public ImageLocation findLocation(String name) { - requireOpen(); - return reader.findLocation(name); - } - - public String[] getEntryNames() { - requireOpen(); - return reader.getEntryNames(); - } - + // TODO: Only called once when processing module info. + // Maybe simplify to use findNode("/modules").getChildren() ? public String[] getModuleNames() { requireOpen(); int off = "/modules/".length(); return reader.findNode("/modules") - .getChildren() - .stream() - .map(Node::getNameString) - .map(s -> s.substring(off, s.length())) - .toArray(String[]::new); - } - - public long[] getAttributes(int offset) { - requireOpen(); - return reader.getAttributes(offset); - } - - public String getString(int offset) { - requireOpen(); - return reader.getString(offset); - } - - public byte[] getResource(String name) { - requireOpen(); - return reader.getResource(name); + .getChildNames() + .map(s -> s.substring(off)) + .toArray(String[]::new); } + // TODO: Called once by JavaURuntimeURLConnection ... public byte[] getResource(ImageLocation loc) { requireOpen(); return reader.getResource(loc); } + // TODO: Called twice in SystemModuleFinders. public ByteBuffer getResourceBuffer(ImageLocation loc) { requireOpen(); return reader.getResourceBuffer(loc); } - public InputStream getResourceStream(ImageLocation loc) { - requireOpen(); - return reader.getResourceStream(loc); - } - private static final class SharedImageReader extends BasicImageReader { - static final int SIZE_OF_OFFSET = Integer.BYTES; - - static final Map OPEN_FILES = new HashMap<>(); + private static final Map OPEN_FILES = new HashMap<>(); // List of openers for this shared image. - final Set openers; + private final Set openers; // attributes of the .jimage file. jimage file does not contain // attributes for the individual resources (yet). We use attributes // of the jimage file itself (creation, modification, access times). - // Iniitalized lazily, see {@link #imageFileAttributes()}. - BasicFileAttributes imageFileAttributes; - - // directory management implementation - final HashMap nodes; - volatile Directory rootDir; + // Initialized lazily, see {@link #imageFileAttributes()}. + private BasicFileAttributes imageFileAttributes; - Directory packagesDir; - Directory modulesDir; + // TODO: Add JavaDoc explaining Image vs System paths. + // Map from absolute "system path" to cached node instances. + // This is guarded by via synchronization of 'this' instance. + private final HashMap nodes; + // Used to quickly test ImageLocation without needing string comparisons. + private final int modulesStringOffset; + private final int packagesStringOffset; private SharedImageReader(Path imagePath, ByteOrder byteOrder) throws IOException { super(imagePath, byteOrder); this.openers = new HashSet<>(); + // TODO: Given there are 30,000 nodes in the image, is setting an initial capacity a good idea? this.nodes = new HashMap<>(); + // The *really* should exist under all circumstances, but there's + // probably a more robust way of getting it... + this.modulesStringOffset = findLocation("/modules/java.base").getModuleOffset(); + this.packagesStringOffset = findLocation("/packages/java.lang").getModuleOffset(); + + // Node creation is very lazy, se can just make the top-level directories + // now without the risk of triggering the building of lots of other nodes. + Directory packages = newDirectory("/packages"); + nodes.put(packages.getName(), packages); + Directory modules = newDirectory("/modules"); + nodes.put(modules.getName(), modules); + + Directory root = newDirectory("/"); + root.setChildren(Arrays.asList(packages, modules)); + nodes.put(root.getName(), root); } public static ImageReader open(Path imagePath, ByteOrder byteOrder) throws IOException { @@ -263,8 +227,8 @@ public void close(ImageReader image) throws IOException { if (openers.isEmpty()) { close(); + // TODO: Should this be synchronized by "this" ?? nodes.clear(); - rootDir = null; if (!OPEN_FILES.remove(this.getImagePath(), this)) { throw new IOException("image file not found in open list"); @@ -273,205 +237,169 @@ public void close(ImageReader image) throws IOException { } } - void addOpener(ImageReader reader) { - synchronized (OPEN_FILES) { - openers.add(reader); - } - } - - boolean removeOpener(ImageReader reader) { - synchronized (OPEN_FILES) { - return openers.remove(reader); + /// Returns a node in the JRT filesystem namespace, or null if no resource or + /// directory exists. + /// + /// Node names are absolute, {@code /}-separated path strings, prefixed with + /// either {@code "/modules"} or {@code "/packages"}. + /// + /// This is the only public API by which anything outside this class can access + /// Node instances either directly, or by resolving a symbolic link. + /// + /// Note also that there is no reentrant calling back to this method from within + /// the node handling code. + synchronized Node findNode(String name) { + Node node = nodes.get(name); + if (node == null) { + node = buildNode(name); + if (node != null) { + nodes.put(node.getName(), node); + } + } else if (!node.isCompleted()) { + // Only directories can be incomplete. + assert node instanceof Directory : "Invalid incomplete node: " + node; + completeDirectory((Directory) node); } + assert node == null || node.isCompleted() : "Incomplete node: " + node; + return node; } - // directory management interface - Directory getRootDirectory() { - return buildRootDirectory(); - } - - /** - * Lazily build a node from a name. - */ - synchronized Node buildNode(String name) { - Node n; + /// Builds a new, "complete" node based on its absolute name. + /// We only get here if the name is not yet present in the nodes map. + private Node buildNode(String name) { + // Zero-allocation test to reject any unexpected paths early. boolean isPackages = name.startsWith("/packages"); boolean isModules = !isPackages && name.startsWith("/modules"); - if (!(isModules || isPackages)) { return null; } - + // Will FAIL for non-directory resources, since the image path does not + // start with "/modules" (e.g. "/java.base/java/lang/Object.class"). ImageLocation loc = findLocation(name); - if (loc != null) { // A sub tree node - if (isPackages) { - n = handlePackages(name, loc); - } else { // modules sub tree - n = handleModulesSubTree(name, loc); - } - } else { // Asking for a resource? /modules/java.base/java/lang/Object.class - if (isModules) { - n = handleResource(name); - } else { - // Possibly ask for /packages/java.lang/java.base - // although /packages/java.base not created - n = handleModuleLink(name); - } + Node node = null; + if (loc != null) { + // A subtree node in either /modules/... or /packages/... + node = isPackages + ? buildPackageNode(name, loc) + : buildModuleDirectory(name, loc); + } else if (isModules) { + // We still cannot trust that this path is valid, but if it is, + // it must be a resource node in /modules/... + node = buildResourceNode(name); } - return n; + return node; } - synchronized Directory buildRootDirectory() { - Directory root = rootDir; // volatile read - if (root != null) { - return root; + /// Completes a directory by ensuring its child list is populated correctly. + private void completeDirectory(Directory dir) { + String name = dir.getName(); + // Since the node exists, we can assert that it's name starts with + // either /modules or /packages, making differentiation easy. It also + // means that the name is valid and which must yield a location. + assert name.startsWith("/modules") || name.startsWith("/packages"); + ImageLocation loc = findLocation(name); + assert loc != null && name.equals(loc.getFullName()) : "Invalid location for name: " + name; + // We cannot use 'isXxxSubdirectory()' methods here since we could + // be given a top-level directory. + if (name.charAt(1) == 'm') { + completeModuleDirectory(dir, loc); + } else { + completePackageDirectory(dir, loc); } - - root = newDirectory(null, "/"); - root.setIsRootDir(); - - // /packages dir - packagesDir = newDirectory(root, "/packages"); - packagesDir.setIsPackagesDir(); - - // /modules dir - modulesDir = newDirectory(root, "/modules"); - modulesDir.setIsModulesDir(); - - root.setCompleted(true); - return rootDir = root; - } - - /** - * To visit sub tree resources. - */ - interface LocationVisitor { - void visit(ImageLocation loc); - } - - void visitLocation(ImageLocation loc, LocationVisitor visitor) { - byte[] offsets = getResource(loc); - ByteBuffer buffer = ByteBuffer.wrap(offsets); - buffer.order(getByteOrder()); - IntBuffer intBuffer = buffer.asIntBuffer(); - for (int i = 0; i < offsets.length / SIZE_OF_OFFSET; i++) { - int offset = intBuffer.get(i); - ImageLocation pkgLoc = getLocation(offset); - visitor.visit(pkgLoc); + assert dir.isCompleted() : "Directory must be complete by now: " + dir; + } + + /// Builds either a 2nd level package directory, or a 3rd level symbolic + /// link from an {@code ImageLocation}. Possible names are: + /// + /// * {@code /packages/}: Builds a {@code Directory}. + /// * {@code /packages//}: Builds a {@code LinkNode}. + /// + /// Called by {@code buildNode()} if a package node is not present in the + /// cache. The top-level {@code /packages} directory was already built by + /// {@code buildRootDirectory}. + private Node buildPackageNode(String name, ImageLocation loc) { + assert !name.equals("/packages") : "Cannot build root /packages directory"; + if (isPackagesSubdirectory(loc)) { + return completePackageDirectory(newDirectory(name), loc); } - } - - void visitPackageLocation(ImageLocation loc) { - // Retrieve package name - String pkgName = getBaseExt(loc); - // Content is array of offsets in Strings table - byte[] stringsOffsets = getResource(loc); - ByteBuffer buffer = ByteBuffer.wrap(stringsOffsets); - buffer.order(getByteOrder()); - IntBuffer intBuffer = buffer.asIntBuffer(); - // For each module, create a link node. - for (int i = 0; i < stringsOffsets.length / SIZE_OF_OFFSET; i++) { - // skip empty state, useless. - intBuffer.get(i); - i++; - int offset = intBuffer.get(i); - String moduleName = getString(offset); - Node targetNode = findNode("/modules/" + moduleName); - if (targetNode != null) { - String pkgDirName = packagesDir.getName() + "/" + pkgName; - Directory pkgDir = (Directory) nodes.get(pkgDirName); - newLinkNode(pkgDir, pkgDir.getName() + "/" + moduleName, targetNode); + int modStart = name.indexOf('/', "/packages/".length()) + 1; + assert modStart > 0 && name.indexOf('/', modStart) == -1 : "Invalid link name: " + name; + return newLinkNode(name, "/modules/" + name.substring(modStart)); + } + + /// Completes a package directory by setting the list of child nodes. + /// + /// This is called by {@code buildPackageNode()}, but also for the + /// top-level {@code /packages} directory. + private Directory completePackageDirectory(Directory dir, ImageLocation loc) { + assert dir.getName().equals(loc.getFullName()) : "Mismatched location for directory: " + dir; + // The only directories in the /packages namespace are /packages or + // /packages/. However, unlike /modules directories, the + // location offsets mean different things. + List children; + if (dir.getName().equals("/packages")) { + // Top-level directory just contains a list of subdirectories. + children = createChildNodes(loc, c -> nodes.computeIfAbsent(c.getFullName(), this::newDirectory)); + } else { + // A package directory's content is array of offset PAIRS in the + // Strings table, but we only need the 2nd value of each pair. + IntBuffer intBuffer = getOffsetBuffer(loc); + int offsetCount = intBuffer.capacity(); + children = new ArrayList<>(offsetCount / 2); + // Iterate the 2nd offset in each pair (odd indices). + for (int i = 1; i < offsetCount; i += 2) { + String moduleName = getString(intBuffer.get(i)); + children.add(nodes.computeIfAbsent( + dir.getName() + "/" + moduleName, + n -> newLinkNode(n, "/modules/" + moduleName))); } } + // This only happens once and "completes" the directory. + dir.setChildren(children); + return dir; } - Node handlePackages(String name, ImageLocation loc) { - long size = loc.getUncompressedSize(); - Node n = null; - // Only possibilities are /packages, /packages/package/module - if (name.equals("/packages")) { - visitLocation(loc, (childloc) -> { - findNode(childloc.getFullName()); - }); - packagesDir.setCompleted(true); - n = packagesDir; - } else { - if (size != 0) { // children are offsets to module in StringsTable - String pkgName = getBaseExt(loc); - Directory pkgDir = newDirectory(packagesDir, packagesDir.getName() + "/" + pkgName); - visitPackageLocation(loc); - pkgDir.setCompleted(true); - n = pkgDir; - } else { // Link to module - String pkgName = loc.getParent(); - String modName = getBaseExt(loc); - Node targetNode = findNode("/modules/" + modName); - if (targetNode != null) { - String pkgDirName = packagesDir.getName() + "/" + pkgName; - Directory pkgDir = (Directory) nodes.get(pkgDirName); - Node linkNode = newLinkNode(pkgDir, pkgDir.getName() + "/" + modName, targetNode); - n = linkNode; - } - } - } - return n; - } - - // Asking for /packages/package/module although - // /packages// not yet created, need to create it - // prior to return the link to module node. - Node handleModuleLink(String name) { - // eg: unresolved /packages/package/module - // Build /packages/package node - Node ret = null; - String radical = "/packages/"; - String path = name; - if (path.startsWith(radical)) { - int start = radical.length(); - int pkgEnd = path.indexOf('/', start); - if (pkgEnd != -1) { - String pkg = path.substring(start, pkgEnd); - String pkgPath = radical + pkg; - Node n = findNode(pkgPath); - // If not found means that this is a symbolic link such as: - // /packages/java.util/java.base/java/util/Vector.class - // and will be done by a retry of the filesystem - for (Node child : n.getChildren()) { - if (child.name.equals(name)) { - ret = child; - break; - } - } - } - } - return ret; - } - - Node handleModulesSubTree(String name, ImageLocation loc) { - Node n; - assert (name.equals(loc.getFullName())); - Directory dir = makeDirectories(name); - visitLocation(loc, (childloc) -> { - String path = childloc.getFullName(); - if (path.startsWith("/modules")) { // a package - makeDirectories(path); - } else { // a resource - makeDirectories(childloc.buildName(true, true, false)); - // if we have already created a resource for this name previously, then don't - // recreate it - if (!nodes.containsKey(childloc.getFullName(true))) { - newResource(dir, childloc); - } + /// Builds a modules subdirectory directory from an {@code ImageLocation}. + /// + /// Called by {@code buildNode()} if a module directory is not present in + /// the cache. The top-level {@code /modules} directory was already built by + /// {@code buildRootDirectory}. + private Node buildModuleDirectory(String name, ImageLocation loc) { + assert name.equals(loc.getFullName()) : "Mismatched location for directory: " + name; + assert isModulesSubdirectory(loc) : "Invalid modules directory: " + name; + return completeModuleDirectory(newDirectory(name), loc); + } + + /// Completes a modules directory by setting the list of child nodes. + /// + /// The given directory can be the top level {@code /modules} directory + /// (so it is NOT safe to use {@code isModulesSubdirectory(loc)} here). + private Directory completeModuleDirectory(Directory dir, ImageLocation loc) { + assert dir.getName().equals(loc.getFullName()) : "Mismatched location for directory: " + dir; + List children = createChildNodes(loc, (childloc) -> { + if (isModulesSubdirectory(childloc)) { + return nodes.computeIfAbsent(childloc.getFullName(), this::newDirectory); + } else { + // Add the "/modules" prefix to image location paths to get + // Jrt file system names. + String resourceName = childloc.getFullName(true); + return nodes.computeIfAbsent(resourceName, n -> newResource(n, childloc)); } }); - dir.setCompleted(true); - n = dir; - return n; + dir.setChildren(children); + return dir; } - Node handleResource(String name) { - Node n = null; + /// Builds a resource node in the {@code /modules} hierarchy. + /// + /// Called by {@code buildNode()} if there was no entry for the given + /// name in the cache. Unlike {@code buildPackageNode()} and + /// {@code buildModuleDirectory()}, there is no ImageLocation (yet) and + /// the given name cannot be trusted to belong to any resource (or even + /// be a valid resource name). + private Node buildResourceNode(String name) { if (!name.startsWith("/modules/")) { return null; } @@ -480,48 +408,80 @@ Node handleResource(String name) { if (moduleEndIndex == -1) { return null; } + // Tests that the implied module name actually exists before during + // a full lookup for the location. + // TODO: I don't think this is strictly necessary (maybe an assert)? ImageLocation moduleLoc = findLocation(name.substring(0, moduleEndIndex)); if (moduleLoc == null || moduleLoc.getModuleOffset() == 0) { return null; } - - String locationPath = name.substring("/modules".length()); - ImageLocation resourceLoc = findLocation(locationPath); - if (resourceLoc != null) { - Directory dir = makeDirectories(resourceLoc.buildName(true, true, false)); - Resource res = newResource(dir, resourceLoc); - n = res; + // Resource paths in the image are NOT prefixed with /modules. + ImageLocation resourceLoc = findLocation(name.substring("/modules".length())); + if (resourceLoc == null) { + return null; + } + return newResource(name, resourceLoc); + } + + /// Creates the list of child nodes for a {@code Directory} based on a + /// given node creation function. + /// + /// Note: This cannot be used for package subdirectories as they have + /// child offsets stored differently to other directories. + private List createChildNodes(ImageLocation loc, Function newChildFn) { + IntBuffer offsets = getOffsetBuffer(loc); + int childCount = offsets.capacity(); + List children = new ArrayList<>(childCount); + for (int i = 0; i < childCount; i++) { + children.add(newChildFn.apply(getLocation(offsets.get(i)))); } - return n; + return children; } - String getBaseExt(ImageLocation loc) { - String base = loc.getBase(); - String ext = loc.getExtension(); - if (ext != null && !ext.isEmpty()) { - base = base + "." + ext; - } - return base; + /// Helper method to extract the integer offset buffer from a directory + /// location. + private IntBuffer getOffsetBuffer(ImageLocation dir) { + assert isDirectory(dir) : "Not a directory: " + dir.getFullName(); + byte[] offsets = getResource(dir); + ByteBuffer buffer = ByteBuffer.wrap(offsets); + buffer.order(getByteOrder()); + return buffer.asIntBuffer(); } - synchronized Node findNode(String name) { - buildRootDirectory(); - Node n = nodes.get(name); - if (n == null || !n.isCompleted()) { - n = buildNode(name); - } - return n; + /// Determines if an image location is a directory in the {@code /modules} + /// namespace (if so, the location name is the Jrt filesystem name). + /// + /// In the image namespace, everything under {@code /modules/} is a + /// directory, and has the same value for {@code getModule()}. + private boolean isModulesSubdirectory(ImageLocation loc) { + return loc.getModuleOffset() == modulesStringOffset; } - /** - * Returns the file attributes of the image file. - */ - BasicFileAttributes imageFileAttributes() { + /// Determines if an image location is a directory in the {@code /packages} + /// namespace (if so, the location name is the Jrt filesystem name). + /// + /// For locations under {@code /packages/} you have both directories and + /// symbolic links. Directories are only at the first level, with symbolic + /// links under them (and symbolic link entries have no content). + private boolean isPackagesSubdirectory(ImageLocation loc) { + return loc.getModuleOffset() == packagesStringOffset + && loc.getUncompressedSize() > 0; + } + + /// Determines if an image location represents a directory of some kind. + /// Directory locations store child offsets in their content. + private boolean isDirectory(ImageLocation loc) { + return isModulesSubdirectory(loc) || isPackagesSubdirectory(loc) + || loc.getFullName().equals("/modules") || loc.getFullName().equals("/packages"); + } + + /// Returns the file attributes of the image file. Currently, all nodes + /// share the same attributes. + private BasicFileAttributes imageFileAttributes() { BasicFileAttributes attrs = imageFileAttributes; if (attrs == null) { try { - Path file = getImagePath(); - attrs = Files.readAttributes(file, BasicFileAttributes.class); + attrs = Files.readAttributes(getImagePath(), BasicFileAttributes.class); } catch (IOException ioe) { throw new UncheckedIOException(ioe); } @@ -530,109 +490,58 @@ BasicFileAttributes imageFileAttributes() { return attrs; } - Directory newDirectory(Directory parent, String name) { - Directory dir = Directory.create(parent, name, imageFileAttributes()); - nodes.put(dir.getName(), dir); - return dir; - } - - Resource newResource(Directory parent, ImageLocation loc) { - Resource res = Resource.create(parent, loc, imageFileAttributes()); - nodes.put(res.getName(), res); - return res; + /// Creates an "incomplete" directory node with no child nodes set. + /// Directories need to be "completed" before they are returned by + /// {@code findNode()}. + private Directory newDirectory(String name) { + return new Directory(name, imageFileAttributes()); } - LinkNode newLinkNode(Directory dir, String name, Node link) { - LinkNode linkNode = LinkNode.create(dir, name, link); - nodes.put(linkNode.getName(), linkNode); - return linkNode; + /// Creates a new resource from an image location. This is the only case + /// where the image location name does not match the requested node name. + /// In image files, resource locations are NOT prefixed by {@code /modules}. + private Resource newResource(String name, ImageLocation loc) { + assert name.equals(loc.getFullName(true)) : "Mismatched location for resource: " + name; + return new Resource(name, loc, imageFileAttributes()); } - Directory makeDirectories(String parent) { - Directory last = rootDir; - for (int offset = parent.indexOf('/', 1); - offset != -1; - offset = parent.indexOf('/', offset + 1)) { - String dir = parent.substring(0, offset); - last = makeDirectory(dir, last); - } - return makeDirectory(parent, last); - + /// Creates a new link node pointing at the given target name. + /// + /// Note that target node is resolved each time {@code resolve()} is + /// called, so if a link node is retained after its reader is closed, + /// it will begin to fail. + private LinkNode newLinkNode(String name, String targetName) { + return new LinkNode(name, () -> findNode(targetName), imageFileAttributes()); } - Directory makeDirectory(String dir, Directory last) { - Directory nextDir = (Directory) nodes.get(dir); - if (nextDir == null) { - nextDir = newDirectory(last, dir); - } - return nextDir; - } - - byte[] getResource(Node node) throws IOException { + /// Returns the content of a resource node. + private byte[] getResource(Node node) throws IOException { + // We could have been given a non-resource node here. + // TODO: Would it be more robust to have this method accept Resource + // and have the caller verify by type (getLocation() fails anyway)? if (node.isResource()) { return super.getResource(node.getLocation()); } throw new IOException("Not a resource: " + node); } - - byte[] getResource(Resource rs) throws IOException { - return super.getResource(rs.getLocation()); - } } - // jimage file does not store directory structure. We build nodes - // using the "path" strings found in the jimage file. - // Node can be a directory or a resource public abstract static class Node { - private static final int ROOT_DIR = 0b0000_0000_0000_0001; - private static final int PACKAGES_DIR = 0b0000_0000_0000_0010; - private static final int MODULES_DIR = 0b0000_0000_0000_0100; - - private int flags; private final String name; private final BasicFileAttributes fileAttrs; - private boolean completed; + // TODO: Only visible for use by ExplodedImage. protected Node(String name, BasicFileAttributes fileAttrs) { this.name = Objects.requireNonNull(name); this.fileAttrs = Objects.requireNonNull(fileAttrs); } /** - * A node is completed when all its direct children have been built. - * - * @return + * A node is completed when all its direct children have been built. As + * such, non-directory nodes are always complete. */ - public boolean isCompleted() { - return completed; - } - - public void setCompleted(boolean completed) { - this.completed = completed; - } - - public final void setIsRootDir() { - flags |= ROOT_DIR; - } - - public final boolean isRootDir() { - return (flags & ROOT_DIR) != 0; - } - - public final void setIsPackagesDir() { - flags |= PACKAGES_DIR; - } - - public final boolean isPackagesDir() { - return (flags & PACKAGES_DIR) != 0; - } - - public final void setIsModulesDir() { - flags |= MODULES_DIR; - } - - public final boolean isModulesDir() { - return (flags & MODULES_DIR) != 0; + boolean isCompleted() { + return true; } public final String getName() { @@ -661,7 +570,7 @@ public boolean isDirectory() { return false; } - public List getChildren() { + public Stream getChildNames() { throw new IllegalArgumentException("not a directory: " + getNameString()); } @@ -685,10 +594,6 @@ public String extension() { return null; } - public long contentOffset() { - return 0L; - } - public final FileTime creationTime() { return fileAttrs.creationTime(); } @@ -729,21 +634,40 @@ public final boolean equals(Object other) { } } - // directory node - directory has full path name without '/' at end. - static final class Directory extends Node { - private final List children; + /// Directory node (referenced from a full path, without a trailing '/'). + /// + /// Directory nodes have two distinct states: + /// * Incomplete: The child list has not been set. + /// * Complete: The child list has been set. + /// + /// When a directory node is returned by {@code findNode()} it is ensured to + /// be complete, but this DOES NOT mean that its child nodes are complete. + /// + /// The users of {@code ImageReader} are expected to account for this and + /// avoid directly traversing the node hierarchy. This isn't as awkward as + /// it seems however, since {@code JrtFileSystem} (the primary user of this + /// API) always represents node locations via {@code JrtPath}, and so will + /// always lookup nodes directly by name rather than traversing the node + /// hierarchy. + /// + /// If it was ever a requirement that {@code getChildren()} would return + /// complete nodes, that could be easily achieved via a memoized child list + /// supplier, but this would likely come at the cost of performance. + /// + /// The exact rule(s) by which the set of child nodes is determined is + /// inferred in `buildNode()` from the structure of the requested path and + /// the underlying `ImageLocation` data. + private static final class Directory extends Node { + // Monotonic reference, will be set to the unmodifiable child list exactly once. + private List children = null; private Directory(String name, BasicFileAttributes fileAttrs) { super(name, fileAttrs); - children = new ArrayList<>(); } - static Directory create(Directory parent, String name, BasicFileAttributes fileAttrs) { - Directory d = new Directory(name, fileAttrs); - if (parent != null) { - parent.addChild(d); - } - return d; + @Override + boolean isCompleted() { + return children != null; } @Override @@ -752,48 +676,37 @@ public boolean isDirectory() { } @Override - public List getChildren() { - return Collections.unmodifiableList(children); - } - - void addChild(Node node) { - assert !children.contains(node) : "Child " + node + " already added"; - children.add(node); + public Stream getChildNames() { + if (children != null) { + return children.stream().map(Node::getName); + } + throw new IllegalStateException("Cannot get child nodes of an incomplete directory: " + getName()); } - public void walk(Consumer consumer) { - consumer.accept(this); - for (Node child : children) { - if (child.isDirectory()) { - ((Directory)child).walk(consumer); - } else { - consumer.accept(child); - } - } + private void setChildren(List children) { + assert this.children == null : this + ": Cannot set child nodes twice!"; + this.children = Collections.unmodifiableList(children); } } - // "resource" is .class or any other resource (compressed/uncompressed) in a jimage. - // full path of the resource is the "name" of the resource. - static class Resource extends Node { + /// Resource node (a ".class" or any other data resource in jimage). + /// + /// Resources are leaf nodes referencing an underlying image location. They + /// are lightweight, and do not cache their contents. + /// + /// Unlike directories (where the node name matches the jimage path for the + /// corresponding {@code ImageLocation}), resource node names are NOT the + /// same as the corresponding jimage path. The difference is simply that + /// jimage paths are not prefixed with "/modules", and just start with + /// their module name (e.g. "/java.base/java/lang/Object.class"). + private static class Resource extends Node { private final ImageLocation loc; - private Resource(ImageLocation loc, BasicFileAttributes fileAttrs) { - super(loc.getFullName(true), fileAttrs); + private Resource(String name, ImageLocation loc, BasicFileAttributes fileAttrs) { + super(name, fileAttrs); this.loc = loc; } - static Resource create(Directory parent, ImageLocation loc, BasicFileAttributes fileAttrs) { - Resource rs = new Resource(loc, fileAttrs); - parent.addChild(rs); - return rs; - } - - @Override - public boolean isCompleted() { - return true; - } - @Override public boolean isResource() { return true; @@ -818,36 +731,24 @@ public long compressedSize() { public String extension() { return loc.getExtension(); } - - @Override - public long contentOffset() { - return loc.getContentOffset(); - } } - // represents a soft link to another Node - static class LinkNode extends Node { - private final Node link; + /// Link node (a symbolic link to a top-level modules directory). + /// + /// Link nodes resolve their target by invoking a given supplier, and do not + /// cache the result. Since nodes are cached by the {@code ImageReader}, this + /// means that only the first call to {@code resolveLink()} can do any work. + private static class LinkNode extends Node { + private final Supplier link; - private LinkNode(String name, Node link) { - super(name, link.getFileAttributes()); + private LinkNode(String name, Supplier link, BasicFileAttributes fileAttrs) { + super(name, fileAttrs); this.link = link; } - static LinkNode create(Directory parent, String name, Node link) { - LinkNode ln = new LinkNode(name, link); - parent.addChild(ln); - return ln; - } - - @Override - public boolean isCompleted() { - return true; - } - @Override public Node resolveLink(boolean recursive) { - return (recursive && link instanceof LinkNode) ? ((LinkNode)link).resolveLink(true) : link; + return link.get(); } @Override diff --git a/src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java b/src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java index e2c17f8ca2513..2925dc2e273f0 100644 --- a/src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java +++ b/src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java @@ -119,7 +119,7 @@ byte[] getContent() throws IOException { } @Override - public List getChildren() { + public Stream getChildNames() { if (!isDirectory()) throw new IllegalArgumentException("not a directory: " + getNameString()); if (children == null) { @@ -138,7 +138,7 @@ public List getChildren() { } children = list; } - return children; + return children.stream().map(Node::getName); } @Override diff --git a/src/java.base/share/classes/jdk/internal/jrtfs/JrtFileSystem.java b/src/java.base/share/classes/jdk/internal/jrtfs/JrtFileSystem.java index 9a8d9d22dfa87..6530bd1f90a6f 100644 --- a/src/java.base/share/classes/jdk/internal/jrtfs/JrtFileSystem.java +++ b/src/java.base/share/classes/jdk/internal/jrtfs/JrtFileSystem.java @@ -54,7 +54,6 @@ import java.nio.file.attribute.FileTime; import java.nio.file.attribute.UserPrincipalLookupService; import java.nio.file.spi.FileSystemProvider; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -64,7 +63,6 @@ import java.util.Set; import java.util.regex.Pattern; import jdk.internal.jimage.ImageReader.Node; -import static java.util.stream.Collectors.toList; /** * jrt file system implementation built on System jimage files. @@ -225,19 +223,19 @@ Iterator iteratorOf(JrtPath path, DirectoryStream.Filter fil throw new NotDirectoryException(path.getName()); } if (filter == null) { - return node.getChildren() - .stream() - .map(child -> (Path)(path.resolve(new JrtPath(this, child.getNameString()).getFileName()))) - .iterator(); + return node.getChildNames() + .map(child -> (Path) (path.resolve(new JrtPath(this, child).getFileName()))) + .iterator(); } - return node.getChildren() - .stream() - .map(child -> (Path)(path.resolve(new JrtPath(this, child.getNameString()).getFileName()))) - .filter(p -> { try { return filter.accept(p); - } catch (IOException x) {} - return false; - }) - .iterator(); + return node.getChildNames() + .map(child -> (Path) (path.resolve(new JrtPath(this, child).getFileName()))) + .filter(p -> { + try { + return filter.accept(p); + } catch (IOException x) {} + return false; + }) + .iterator(); } // returns the content of the file resource specified by the path diff --git a/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java b/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java index c520e6e636aa2..3c27f06979222 100644 --- a/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java +++ b/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java @@ -34,7 +34,6 @@ import java.lang.module.ModuleReference; import java.lang.reflect.Constructor; import java.net.URI; -import java.net.URLConnection; import java.nio.ByteBuffer; import java.nio.file.Files; import java.nio.file.Path; @@ -481,7 +480,7 @@ public void close() { private static class ModuleContentSpliterator implements Spliterator { final String moduleRoot; final Deque stack; - Iterator iterator; + Iterator iterator; ModuleContentSpliterator(String module) throws IOException { moduleRoot = "/modules/" + module; @@ -502,13 +501,10 @@ private static class ModuleContentSpliterator implements Spliterator { private String next() throws IOException { for (;;) { while (iterator.hasNext()) { - ImageReader.Node node = iterator.next(); - String name = node.getName(); + String name = iterator.next(); + ImageReader.Node node = SystemImage.reader().findNode(name); if (node.isDirectory()) { - // build node - ImageReader.Node dir = SystemImage.reader().findNode(name); - assert dir.isDirectory(); - stack.push(dir); + stack.push(node); } else { // strip /modules/$MODULE/ prefix return name.substring(moduleRoot.length() + 1); @@ -520,7 +516,7 @@ private String next() throws IOException { } else { ImageReader.Node dir = stack.poll(); assert dir.isDirectory(); - iterator = dir.getChildren().iterator(); + iterator = dir.getChildNames().iterator(); } } } diff --git a/test/jdk/jdk/internal/jimage/JImageReadTest.java b/test/jdk/jdk/internal/jimage/JImageReadTest.java index ea700d03a4f18..157262e42d332 100644 --- a/test/jdk/jdk/internal/jimage/JImageReadTest.java +++ b/test/jdk/jdk/internal/jimage/JImageReadTest.java @@ -49,6 +49,9 @@ import org.testng.Assert; import org.testng.TestNG; +import static java.nio.ByteOrder.BIG_ENDIAN; +import static java.nio.ByteOrder.LITTLE_ENDIAN; + @Test public class JImageReadTest { @@ -333,32 +336,23 @@ static void test4_nameTooLong() throws IOException { */ @Test static void test5_imageReaderEndianness() throws IOException { - ImageReader nativeReader = ImageReader.open(imageFile); - Assert.assertEquals(nativeReader.getByteOrder(), ByteOrder.nativeOrder()); - - try { - ImageReader leReader = ImageReader.open(imageFile, ByteOrder.LITTLE_ENDIAN); - Assert.assertEquals(leReader.getByteOrder(), ByteOrder.LITTLE_ENDIAN); - leReader.close(); - } catch (IOException io) { - // IOException expected if LITTLE_ENDIAN not the nativeOrder() - Assert.assertNotEquals(ByteOrder.nativeOrder(), ByteOrder.LITTLE_ENDIAN); + // Will be opened with native byte order. + try (ImageReader nativeReader = ImageReader.open(imageFile)) { + // Just ensure something works as expected. + Assert.assertNotNull(nativeReader.getRootDirectory()); + } catch (IOException expected) { + Assert.fail("Reader should be openable with native byte order."); } - try { - ImageReader beReader = ImageReader.open(imageFile, ByteOrder.BIG_ENDIAN); - Assert.assertEquals(beReader.getByteOrder(), ByteOrder.BIG_ENDIAN); - beReader.close(); - } catch (IOException io) { - // IOException expected if LITTLE_ENDIAN not the nativeOrder() - Assert.assertNotEquals(ByteOrder.nativeOrder(), ByteOrder.BIG_ENDIAN); + ByteOrder otherOrder = ByteOrder.nativeOrder() == BIG_ENDIAN ? LITTLE_ENDIAN : BIG_ENDIAN; + try (ImageReader badReader = ImageReader.open(imageFile, otherOrder)) { + Assert.fail("Reader should not be openable with the wrong byte order."); + } catch (IOException expected) { } - - nativeReader.close(); } - // main method to run standalone from jtreg - @Test(enabled=false) + // main method to run standalone from jtreg + @Test(enabled = false) @Parameters({"x"}) @SuppressWarnings("raw_types") public static void main(@Optional String[] args) { From 37642fe7184d43f700992b7db120f2ae7e634a45 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Wed, 18 Jun 2025 17:44:10 +0200 Subject: [PATCH 02/11] WIP - more tests needed. * Stacking JDK-8359808 (dependent PR) changes on top temporarily. * Removing the last misleading/incorrect API from ImageReader. * WIP - looking at adding tests. * Removed all APIs which leak ImageLocation to ImageReader users. --- .../jdk/internal/jimage/ImageReader.java | 165 ++++++++++-------- .../jdk/internal/jrtfs/SystemImage.java | 1 - .../internal/module/SystemModuleFinders.java | 93 +++++----- .../jrt/JavaRuntimeURLConnection.java | 143 ++++++--------- test/jdk/sun/net/www/protocol/jrt/Basic.java | 27 ++- 5 files changed, 224 insertions(+), 205 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java b/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java index 5c22a05c1d386..3b3c3b90edf31 100644 --- a/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java +++ b/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 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 @@ -47,6 +47,32 @@ import java.util.stream.Stream; /** + * An adapter for {@link BasicImageReader} to present jimage resources in a + * file system friendly way. jimage entries (resources, module and package + * information) are mapped into a unified hierarchy of named nodes, which serve + * as the underlying structure for the JrtFileSystem implementation and other + * utilities. + * + *

Because this class is adapting underlying jimage semantics, there is no + * need for it to export all the features and behaviour of that class (it is not + * a conceptual subtype of {@code BasicImageReader}). {@code ImageReader}'s job + * is to support whatever is needed by internal tools which want a file system + * like view of jimage resources. + * + *

Entries in jimage are exported as one of three {@link Node} types; + * resource nodes, directory nodes and link nodes. + * + *

When remapping jimage entries, jimage location names (e.g. {@code + * "/java.base/java/lang/Integer.class"}) are prefixed with {@code "/modules"} + * to form the names of resource nodes. This aligns with the naming of module + * entries in jimage (e.g. "/modules/java.base/java/lang"), which appear as + * directory nodes in {@code ImageReader}. + * + *

Package entries (e.g. {@code "/packages/java.lang/java.base"} appear as + * link nodes, pointing back to the root directory of the associated module in + * which the package exists (e.g. {@code "/modules/java.base"}). This provides + * a mechanism compatible with the notion of symbolic links in a file system. + * * @implNote This class needs to maintain JDK 8 source compatibility. * * It is used internally in the JDK to implement jimage/jrtfs access, @@ -62,6 +88,14 @@ private ImageReader(SharedImageReader reader) { this.reader = reader; } + /** + * Opens an image reader for a jimage file at the specified path, using the + * given byte order. + * + *

Almost all callers should use {@link #open(Path)} to obtain a reader + * with the platform native byte ordering. Using a non-native ordering is + * extremely unusual. + */ public static ImageReader open(Path imagePath, ByteOrder byteOrder) throws IOException { Objects.requireNonNull(imagePath); Objects.requireNonNull(byteOrder); @@ -69,6 +103,13 @@ public static ImageReader open(Path imagePath, ByteOrder byteOrder) throws IOExc return SharedImageReader.open(imagePath, byteOrder); } + /** + * Opens an image reader for a jimage file at the specified path, using the + * platform native byte order. + * + *

This is the expected was to open an {@code ImageReader}, and it should + * be rare for anything other than the native byte order to be needed. + */ public static ImageReader open(Path imagePath) throws IOException { return open(imagePath, ByteOrder.nativeOrder()); } @@ -94,63 +135,51 @@ private void requireOpen() { } } - // TODO: Called by SystemImage (now completely pointless). - // directory management interface - public Directory getRootDirectory() throws IOException { - ensureOpen(); - return (Directory) findNode("/"); - } - - // TODO: Called by SystemImage and SystemModuleFinders. + /** + * Finds the node for the given JRT file system name. + * + * @param name a JRT file system name (path) of the form + * {@code "/modules//...} or {@code "/packages//...}. + * @return a node representing a resource, directory or symbolic link. + */ public Node findNode(String name) throws IOException { ensureOpen(); return reader.findNode(name); } - // TODO: Called by SystemImage for JrtFileSystem. + /** + * Returns the content of a resource node. + * + * @throws IOException if the content cannot be returned (including if the + * given node is not a resource node). + */ public byte[] getResource(Node node) throws IOException { ensureOpen(); return reader.getResource(node); } - // TODO: Called once from SystemModuleReader::release(). + /** + * Releases a (possibly cached) {@link ByteBuffer} obtained via + * {@link #getResourceBuffer(Node)}. + * + *

Note that no testing is performed to check whether the buffer about + * to be released actually came from a call to {@code getResourceBuffer()}. + */ public static void releaseByteBuffer(ByteBuffer buffer) { BasicImageReader.releaseByteBuffer(buffer); } - // TODO: Avoid leaking ImageLocation into callers (only 3) so we can - // encapsulate better for exploded image etc. - public ImageLocation findLocation(String mn, String rn) { - requireOpen(); - return reader.findLocation(mn, rn); - } - - public boolean verifyLocation(String mn, String rn) { + /** + * Returns the content of a resource node in a possibly cached byte buffer. + * Callers of this method must call {@link #releaseByteBuffer(ByteBuffer)} + * when they are finished with it. + */ + public ByteBuffer getResourceBuffer(Node node) { requireOpen(); - return reader.verifyLocation(mn, rn); - } - - // TODO: Only called once when processing module info. - // Maybe simplify to use findNode("/modules").getChildren() ? - public String[] getModuleNames() { - requireOpen(); - int off = "/modules/".length(); - return reader.findNode("/modules") - .getChildNames() - .map(s -> s.substring(off)) - .toArray(String[]::new); - } - - // TODO: Called once by JavaURuntimeURLConnection ... - public byte[] getResource(ImageLocation loc) { - requireOpen(); - return reader.getResource(loc); - } - - // TODO: Called twice in SystemModuleFinders. - public ByteBuffer getResourceBuffer(ImageLocation loc) { - requireOpen(); - return reader.getResourceBuffer(loc); + if (!node.isResource()) { + throw new IllegalStateException("Not a resource node: " + node); + } + return reader.getResourceBuffer(node.getLocation()); } private static final class SharedImageReader extends BasicImageReader { @@ -165,8 +194,6 @@ private static final class SharedImageReader extends BasicImageReader { // Initialized lazily, see {@link #imageFileAttributes()}. private BasicFileAttributes imageFileAttributes; - // TODO: Add JavaDoc explaining Image vs System paths. - // Map from absolute "system path" to cached node instances. // This is guarded by via synchronization of 'this' instance. private final HashMap nodes; // Used to quickly test ImageLocation without needing string comparisons. @@ -517,8 +544,6 @@ private LinkNode newLinkNode(String name, String targetName) { /// Returns the content of a resource node. private byte[] getResource(Node node) throws IOException { // We could have been given a non-resource node here. - // TODO: Would it be more robust to have this method accept Resource - // and have the caller verify by type (getLocation() fails anyway)? if (node.isResource()) { return super.getResource(node.getLocation()); } @@ -530,16 +555,14 @@ public abstract static class Node { private final String name; private final BasicFileAttributes fileAttrs; - // TODO: Only visible for use by ExplodedImage. + // Only visible for use by ExplodedImage. protected Node(String name, BasicFileAttributes fileAttrs) { this.name = Objects.requireNonNull(name); this.fileAttrs = Objects.requireNonNull(fileAttrs); } - /** - * A node is completed when all its direct children have been built. As - * such, non-directory nodes are always complete. - */ + // A node is completed when all its direct children have been built. As + // such, non-directory nodes are always complete. boolean isCompleted() { return true; } @@ -640,23 +663,18 @@ public final boolean equals(Object other) { /// * Incomplete: The child list has not been set. /// * Complete: The child list has been set. /// - /// When a directory node is returned by {@code findNode()} it is ensured to - /// be complete, but this DOES NOT mean that its child nodes are complete. - /// - /// The users of {@code ImageReader} are expected to account for this and - /// avoid directly traversing the node hierarchy. This isn't as awkward as - /// it seems however, since {@code JrtFileSystem} (the primary user of this - /// API) always represents node locations via {@code JrtPath}, and so will - /// always lookup nodes directly by name rather than traversing the node - /// hierarchy. + /// When a directory node is returned by `findNode()` it is always complete, + /// but this DOES NOT mean that its child nodes are complete yet. /// - /// If it was ever a requirement that {@code getChildren()} would return - /// complete nodes, that could be easily achieved via a memoized child list - /// supplier, but this would likely come at the cost of performance. + /// To avoid users being able to access incomplete child nodes, the + /// {@link Node} API offers only a way to obtain child node names, forcing + /// callers to invoke {@link ImageReader#findNode(String)} if they need to + /// access the child node itself. /// - /// The exact rule(s) by which the set of child nodes is determined is - /// inferred in `buildNode()` from the structure of the requested path and - /// the underlying `ImageLocation` data. + /// This approach allows directories to be implemented lazily with respect + /// to child nodes, while retaining efficiency when child nodes are accessed + /// (since the incomplete node will be created and placed in the node cache + /// as the parent is accessed). private static final class Directory extends Node { // Monotonic reference, will be set to the unmodifiable child list exactly once. private List children = null; @@ -695,10 +713,10 @@ private void setChildren(List children) { /// are lightweight, and do not cache their contents. /// /// Unlike directories (where the node name matches the jimage path for the - /// corresponding {@code ImageLocation}), resource node names are NOT the - /// same as the corresponding jimage path. The difference is simply that - /// jimage paths are not prefixed with "/modules", and just start with - /// their module name (e.g. "/java.base/java/lang/Object.class"). + /// corresponding `ImageLocation`), resource node names are NOT the same as + /// the corresponding jimage path. The difference is that node names for + /// resources are prefixed with "/modules", which is missing from the + /// equivalent jimage path. private static class Resource extends Node { private final ImageLocation loc; @@ -736,8 +754,9 @@ public String extension() { /// Link node (a symbolic link to a top-level modules directory). /// /// Link nodes resolve their target by invoking a given supplier, and do not - /// cache the result. Since nodes are cached by the {@code ImageReader}, this - /// means that only the first call to {@code resolveLink()} can do any work. + /// cache the result. Since nodes are cached by the `ImageReader`, this + /// means that only the first call to `resolveLink()` could do any + /// significant work. private static class LinkNode extends Node { private final Supplier link; diff --git a/src/java.base/share/classes/jdk/internal/jrtfs/SystemImage.java b/src/java.base/share/classes/jdk/internal/jrtfs/SystemImage.java index 88cdf724e7d39..6813c7e627f2e 100644 --- a/src/java.base/share/classes/jdk/internal/jrtfs/SystemImage.java +++ b/src/java.base/share/classes/jdk/internal/jrtfs/SystemImage.java @@ -58,7 +58,6 @@ static SystemImage open() throws IOException { if (modulesImageExists) { // open a .jimage and build directory structure final ImageReader image = ImageReader.open(moduleImageFile); - image.getRootDirectory(); return new SystemImage() { @Override Node findNode(String path) throws IOException { diff --git a/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java b/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java index 3c27f06979222..6eaf5e7a2819b 100644 --- a/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java +++ b/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 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 @@ -44,7 +44,6 @@ import java.util.HashSet; import java.util.Iterator; import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.Spliterator; @@ -53,7 +52,6 @@ import java.util.stream.Stream; import java.util.stream.StreamSupport; -import jdk.internal.jimage.ImageLocation; import jdk.internal.jimage.ImageReader; import jdk.internal.jimage.ImageReaderFactory; import jdk.internal.access.JavaNetUriAccess; @@ -61,6 +59,8 @@ import jdk.internal.util.StaticProperty; import jdk.internal.module.ModuleHashes.HashSupplier; +import static java.util.Objects.requireNonNull; + /** * The factory for SystemModules objects and for creating ModuleFinder objects * that find modules in the runtime image. @@ -218,20 +218,16 @@ private static ModuleFinder ofModuleInfos() { // parse the module-info.class in every module Map nameToAttributes = new HashMap<>(); Map nameToHash = new HashMap<>(); - ImageReader reader = SystemImage.reader(); - for (String mn : reader.getModuleNames()) { - ImageLocation loc = reader.findLocation(mn, "module-info.class"); - ModuleInfo.Attributes attrs - = ModuleInfo.read(reader.getResourceBuffer(loc), null); - nameToAttributes.put(mn, attrs); + getModuleAttributes().forEach(attrs -> { + nameToAttributes.put(attrs.descriptor().name(), attrs); ModuleHashes hashes = attrs.recordedHashes(); if (hashes != null) { for (String name : hashes.names()) { nameToHash.computeIfAbsent(name, k -> hashes.hashFor(name)); } } - } + }); // create a ModuleReference for each module Set mrefs = new HashSet<>(); @@ -252,6 +248,26 @@ private static ModuleFinder ofModuleInfos() { return new SystemModuleFinder(mrefs, nameToModule); } + private static Stream getModuleAttributes() { + // System reader is a singleton and should not be closed by callers. + ImageReader reader = SystemImage.reader(); + return getNode(reader, "/modules") + .getChildNames() + .map(mn -> getNode(reader, mn + "/module-info.class")) + // This fails with ISE if the node isn't a resource (corrupt JImage). + .map(reader::getResourceBuffer) + .map(bb -> ModuleInfo.read(bb, null)); + } + + // The nodes we are processing must exist (every module must have a module-info.class). + private static ImageReader.Node getNode(ImageReader reader, String name) { + try { + return requireNonNull(reader.findNode(name)); + } catch (IOException e) { + throw new IllegalStateException("ImageReader node must exist: " + name, e); + } + } + /** * A ModuleFinder that finds module in an array or set of modules. */ @@ -273,7 +289,7 @@ private static class SystemModuleFinder implements ModuleFinder { @Override public Optional find(String name) { - Objects.requireNonNull(name); + requireNonNull(name); return Optional.ofNullable(nameToModule.get(name)); } @@ -381,34 +397,18 @@ private static class SystemModuleReader implements ModuleReader { this.module = module; } - /** - * Returns the ImageLocation for the given resource, {@code null} - * if not found. - */ - private ImageLocation findImageLocation(String name) throws IOException { - Objects.requireNonNull(name); - if (closed) - throw new IOException("ModuleReader is closed"); - ImageReader imageReader = SystemImage.reader(); - if (imageReader != null) { - return imageReader.findLocation(module, name); - } else { - // not an images build - return null; - } - } - /** * Returns {@code true} if the given resource exists, {@code false} * if not found. */ - private boolean containsImageLocation(String name) throws IOException { - Objects.requireNonNull(name); + private boolean containsResource(String resourcePath) throws IOException { + requireNonNull(resourcePath); if (closed) throw new IOException("ModuleReader is closed"); ImageReader imageReader = SystemImage.reader(); if (imageReader != null) { - return imageReader.verifyLocation(module, name); + ImageReader.Node node = imageReader.findNode("/modules" + resourcePath); + return node != null && node.isResource(); } else { // not an images build return false; @@ -417,8 +417,9 @@ private boolean containsImageLocation(String name) throws IOException { @Override public Optional find(String name) throws IOException { - if (containsImageLocation(name)) { - URI u = JNUA.create("jrt", "/" + module + "/" + name); + String resourcePath = "/" + module + "/" + name; + if (containsResource(resourcePath)) { + URI u = JNUA.create("jrt", resourcePath); return Optional.of(u); } else { return Optional.empty(); @@ -441,19 +442,31 @@ private InputStream toInputStream(ByteBuffer bb) { // ## -> ByteBuffer? } } + /** + * Returns the node for the given resource, or {@code null} if not found. + */ + private Optional findResourceNode(ImageReader reader, String name) throws IOException { + requireNonNull(name); + if (closed) { + throw new IOException("ModuleReader is closed"); + } + String nodeName = "/modules/" + module + "/" + name; + Optional node = Optional.ofNullable(reader.findNode(nodeName)); + if (node.isPresent() && !node.get().isResource()) { + throw new IllegalStateException("Not a resource node: " + node.get()); + } + return node; + } + @Override public Optional read(String name) throws IOException { - ImageLocation location = findImageLocation(name); - if (location != null) { - return Optional.of(SystemImage.reader().getResourceBuffer(location)); - } else { - return Optional.empty(); - } + ImageReader reader = SystemImage.reader(); + return findResourceNode(reader, name).map(reader::getResourceBuffer); } @Override public void release(ByteBuffer bb) { - Objects.requireNonNull(bb); + requireNonNull(bb); ImageReader.releaseByteBuffer(bb); } diff --git a/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java b/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java index 4e67c962a46e1..92a895868f40d 100644 --- a/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java +++ b/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2024, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 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 @@ -31,11 +31,10 @@ import java.net.MalformedURLException; import java.net.URL; -import jdk.internal.jimage.ImageLocation; import jdk.internal.jimage.ImageReader; +import jdk.internal.jimage.ImageReader.Node; import jdk.internal.jimage.ImageReaderFactory; -import jdk.internal.loader.Resource; import sun.net.www.ParseUtil; import sun.net.www.URLConnection; @@ -45,99 +44,79 @@ */ public class JavaRuntimeURLConnection extends URLConnection { - // ImageReader to access resources in jimage - private static final ImageReader reader = ImageReaderFactory.getImageReader(); - - // the module and resource name in the URL + // ImageReader to access resources in jimage (never null). + private static final ImageReader READER = ImageReaderFactory.getImageReader(); + + // The module and resource name in the URL ("jrt://"). + // + // It is important to note that all of this information comes from the given + // URL's path part, and there's no requirement for there to be distinct rules + // about percent encoding. However, we choose to treat the module name is if + // it were a URL authority (since Java package/module names are historically + // strongly associated with internet domain names). + // + // The module name is not percent-decoded, and can be empty. private final String module; + // The resource name permits UTF-8 percent encoding of non-ASCII characters. private final String name; - // the Resource when connected - private volatile Resource resource; + // The resource node (when connected). + private volatile Node resource; JavaRuntimeURLConnection(URL url) throws IOException { super(url); - String path = url.getPath(); - if (path.isEmpty() || path.charAt(0) != '/') + // TODO: Reject module names which appear to be using percent encoding. + // TODO: Consider rejecting URLs with fragments, queries or authority. + String urlPath = url.getPath(); + if (urlPath.isEmpty() || urlPath.charAt(0) != '/') { throw new MalformedURLException(url + " missing path or /"); - if (path.length() == 1) { - this.module = null; + } + int pathSep = urlPath.indexOf('/', 1); + if (pathSep == -1) { + // No trailing resource path. This can never "connect" or return a + // resource, but might be useful as a representation to pass around. + // The module name *can* be empty here (e.g. "jrt:/") but not null. + this.module = urlPath.substring(1); this.name = null; } else { - int pos = path.indexOf('/', 1); - if (pos == -1) { - this.module = path.substring(1); - this.name = null; - } else { - this.module = path.substring(1, pos); - this.name = ParseUtil.decode(path.substring(pos+1)); - } + this.module = urlPath.substring(1, pathSep); + this.name = percentDecode(urlPath.substring(pathSep + 1)); } } /** - * Finds a resource in a module, returning {@code null} if the resource - * is not found. + * Finds and caches the resource node associated with this URL and marks the + * connection as "connected". */ - private static Resource findResource(String module, String name) { - if (reader != null) { - URL url = toJrtURL(module, name); - ImageLocation location = reader.findLocation(module, name); - if (location != null) { - return new Resource() { - @Override - public String getName() { - return name; - } - @Override - public URL getURL() { - return url; - } - @Override - public URL getCodeSourceURL() { - return toJrtURL(module); - } - @Override - public InputStream getInputStream() throws IOException { - byte[] resource = reader.getResource(location); - return new ByteArrayInputStream(resource); - } - @Override - public int getContentLength() { - long size = location.getUncompressedSize(); - return (size > Integer.MAX_VALUE) ? -1 : (int) size; - } - }; + private synchronized Node getResourceNode() throws IOException { + if (resource == null) { + if (name == null) { + throw new IOException("cannot connect to jrt:/" + module); } + Node node = READER.findNode("/modules/" + module + "/" + name); + if (node == null || !node.isResource()) { + throw new IOException(module + "/" + name + " not found"); + } + this.resource = node; + super.connected = true; } - return null; + return resource; } @Override - public synchronized void connect() throws IOException { - if (!connected) { - if (name == null) { - String s = (module == null) ? "" : module; - throw new IOException("cannot connect to jrt:/" + s); - } - resource = findResource(module, name); - if (resource == null) - throw new IOException(module + "/" + name + " not found"); - connected = true; - } + public void connect() throws IOException { + getResourceNode(); } @Override public InputStream getInputStream() throws IOException { - connect(); - return resource.getInputStream(); + return new ByteArrayInputStream(READER.getResource(getResourceNode())); } @Override public long getContentLengthLong() { try { - connect(); - return resource.getContentLength(); + return getResourceNode().size(); } catch (IOException ioe) { return -1L; } @@ -149,27 +128,17 @@ public int getContentLength() { return len > Integer.MAX_VALUE ? -1 : (int)len; } - /** - * Returns a jrt URL for the given module and resource name. - */ - @SuppressWarnings("deprecation") - private static URL toJrtURL(String module, String name) { - try { - return new URL("jrt:/" + module + "/" + name); - } catch (MalformedURLException e) { - throw new InternalError(e); + // Perform percent decoding of the resource name/path from the URL. + private static String percentDecode(String path) throws MalformedURLException { + if (path.indexOf('%') == -1) { + // Nothing to decode (overwhelmingly common case). + return path; } - } - - /** - * Returns a jrt URL for the given module. - */ - @SuppressWarnings("deprecation") - private static URL toJrtURL(String module) { + // TODO: Reject over-encoded paths, especially %2F (/) and %24 ($). try { - return new URL("jrt:/" + module); - } catch (MalformedURLException e) { - throw new InternalError(e); + return ParseUtil.decode(path); + } catch (IllegalArgumentException e) { + throw new MalformedURLException(e.getMessage()); } } } diff --git a/test/jdk/sun/net/www/protocol/jrt/Basic.java b/test/jdk/sun/net/www/protocol/jrt/Basic.java index 0f0492c9b39dc..3778912c375e5 100644 --- a/test/jdk/sun/net/www/protocol/jrt/Basic.java +++ b/test/jdk/sun/net/www/protocol/jrt/Basic.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 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 @@ -28,7 +28,6 @@ */ import java.io.IOException; -import java.io.InputStream; import java.net.URL; import java.net.URLConnection; @@ -41,8 +40,28 @@ public class Basic { @DataProvider(name = "urls") public Object[][] urls() { Object[][] data = { - { "jrt:/java.base/java/lang/Object.class", true }, - { "jrt:/java.desktop/java/lang/Object.class", false }, + {"jrt:/java.base/java/lang/Object.class", true}, + // Valid resource with and without percent-encoding. + {"jrt:/java.base/java/lang/Runtime$Version.class", true}, + {"jrt:/java.base/java%2Flang%2FRuntime%24Version.class", true}, + // Unnecessary percent encoding (just Object again). + {"jrt:/java.base/%6a%61%76%61%2f%6c%61%6e%67%2f%4f%62%6a%65%63%74%2e%63%6c%61%73%73", true}, + // Query parameters and fragments are silently ignored. + {"jrt:/java.base/java/lang/Object.class?yes=no", true}, + {"jrt:/java.base/java/lang/Object.class#anchor", true}, + + // Missing resource (no such class). + {"jrt:/java.base/java/lang/NoSuchClass.class", false}, + // Missing resource (wrong module). + {"jrt:/java.desktop/java/lang/Object.class", false}, + // Entries in jimage which don't reference resources. + {"jrt:/modules/java.base/java/lang", false}, + {"jrt:/packages/java.lang", false}, + // Invalid (incomplete/corrupt) URIs. + {"jrt:/java.base", false}, + {"jrt:/java.base/", false}, + // Cannot escape anything in the module name. + {"jrt:/java%2Ebase/java/lang/Object.class", false}, }; return data; } From 359314033168303e432819205652694d6ee37982 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Sat, 28 Jun 2025 01:04:38 +0200 Subject: [PATCH 03/11] Pretty much ready for review now. * Tidying comments, removing dead methods and adding more todos for review discussion. * Moving and merging methods for better ordering, and tidying comments. * Tidying tests. * Reworking to fix symbolic link node creation. * Minor tidy up of comments. * Updating tests for new API. * Stacking JDK-8359808 (dependent PR) changes on top temporarily. --- .../jdk/internal/jimage/ImageReader.java | 401 +++++++++--------- .../jdk/internal/jrtfs/ExplodedImage.java | 2 +- .../jdk/internal/jrtfs/JrtFileAttributes.java | 6 +- .../jrt/JavaRuntimeURLConnection.java | 11 +- .../jdk/internal/jimage/ImageReaderTest.java | 267 ++++++++++++ .../jdk/internal/jimage/JImageReadTest.java | 4 +- .../ImageReaderDuplicateChildNodesTest.java | 10 +- 7 files changed, 490 insertions(+), 211 deletions(-) create mode 100644 test/jdk/jdk/internal/jimage/ImageReaderTest.java diff --git a/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java b/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java index 3b3c3b90edf31..97ef47d7ca2e5 100644 --- a/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java +++ b/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java @@ -30,9 +30,8 @@ import java.nio.ByteOrder; import java.nio.IntBuffer; import java.nio.file.Files; -import java.nio.file.attribute.BasicFileAttributes; -import java.nio.file.attribute.FileTime; import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -48,19 +47,14 @@ /** * An adapter for {@link BasicImageReader} to present jimage resources in a - * file system friendly way. jimage entries (resources, module and package + * file system friendly way. The jimage entries (resources, module and package * information) are mapped into a unified hierarchy of named nodes, which serve - * as the underlying structure for the JrtFileSystem implementation and other - * utilities. + * as the underlying structure for the {@code JrtFileSystem} and other utilities. * - *

Because this class is adapting underlying jimage semantics, there is no - * need for it to export all the features and behaviour of that class (it is not - * a conceptual subtype of {@code BasicImageReader}). {@code ImageReader}'s job - * is to support whatever is needed by internal tools which want a file system - * like view of jimage resources. - * - *

Entries in jimage are exported as one of three {@link Node} types; - * resource nodes, directory nodes and link nodes. + *

This class is not a conceptual subtype of {@code BasicImageReader} and + * deliberately hides types such as {@code ImageLocation} to give a focused API + * based only on the {@link Node} type. Entries in jimage are expressed as one + * of three {@link Node} types resource nodes, directory nodes and link nodes. * *

When remapping jimage entries, jimage location names (e.g. {@code * "/java.base/java/lang/Integer.class"}) are prefixed with {@code "/modules"} @@ -68,10 +62,12 @@ * entries in jimage (e.g. "/modules/java.base/java/lang"), which appear as * directory nodes in {@code ImageReader}. * - *

Package entries (e.g. {@code "/packages/java.lang/java.base"} appear as - * link nodes, pointing back to the root directory of the associated module in - * which the package exists (e.g. {@code "/modules/java.base"}). This provides - * a mechanism compatible with the notion of symbolic links in a file system. + *

Package entries (e.g. {@code "/packages/java.lang"} appear as directory + * nodes containing link nodes, which resolve back to the root directory of the + * module in which that package exists (e.g. {@code "/modules/java.base"}). + * Unlike other nodes, the jimage file does not contain explicit entries for + * link nodes, and their existence is derived only from the contents of the + * parent directory. * * @implNote This class needs to maintain JDK 8 source compatibility. * @@ -184,37 +180,39 @@ public ByteBuffer getResourceBuffer(Node node) { private static final class SharedImageReader extends BasicImageReader { private static final Map OPEN_FILES = new HashMap<>(); + public static final String MODULES_ROOT = "/modules"; + public static final String PACKAGES_ROOT = "/packages"; // List of openers for this shared image. private final Set openers; - // attributes of the .jimage file. jimage file does not contain + // Attributes of the .jimage file. The jimage file does not contain // attributes for the individual resources (yet). We use attributes // of the jimage file itself (creation, modification, access times). - // Initialized lazily, see {@link #imageFileAttributes()}. + // Initialized lazily, see imageFileAttributes(). private BasicFileAttributes imageFileAttributes; - // This is guarded by via synchronization of 'this' instance. + // Guarded by synchronizing 'this' instance. private final HashMap nodes; - // Used to quickly test ImageLocation without needing string comparisons. + // Used to classify ImageLocation instances without string comparison. private final int modulesStringOffset; private final int packagesStringOffset; private SharedImageReader(Path imagePath, ByteOrder byteOrder) throws IOException { super(imagePath, byteOrder); this.openers = new HashSet<>(); - // TODO: Given there are 30,000 nodes in the image, is setting an initial capacity a good idea? + // TODO (review note): Given there are ~30,000 nodes in the image, is setting an initial capacity a good idea? this.nodes = new HashMap<>(); - // The *really* should exist under all circumstances, but there's - // probably a more robust way of getting it... + // TODO (review note): These should exist under all circumstances, but there's + // probably a more robust way of getting it these offsets. this.modulesStringOffset = findLocation("/modules/java.base").getModuleOffset(); this.packagesStringOffset = findLocation("/packages/java.lang").getModuleOffset(); // Node creation is very lazy, se can just make the top-level directories // now without the risk of triggering the building of lots of other nodes. - Directory packages = newDirectory("/packages"); + Directory packages = newDirectory(PACKAGES_ROOT); nodes.put(packages.getName(), packages); - Directory modules = newDirectory("/modules"); + Directory modules = newDirectory(MODULES_ROOT); nodes.put(modules.getName(), modules); Directory root = newDirectory("/"); @@ -254,7 +252,7 @@ public void close(ImageReader image) throws IOException { if (openers.isEmpty()) { close(); - // TODO: Should this be synchronized by "this" ?? + // TODO (review note): Should this be synchronized by "this" ?? nodes.clear(); if (!OPEN_FILES.remove(this.getImagePath(), this)) { @@ -265,20 +263,26 @@ public void close(ImageReader image) throws IOException { } /// Returns a node in the JRT filesystem namespace, or null if no resource or - /// directory exists. + /// directory of that name exists. /// - /// Node names are absolute, {@code /}-separated path strings, prefixed with - /// either {@code "/modules"} or {@code "/packages"}. + /// Node names are absolute, `/`-separated path strings, prefixed with + /// either "/modules" or "/packages". /// /// This is the only public API by which anything outside this class can access - /// Node instances either directly, or by resolving a symbolic link. + /// `Node` instances either directly, or by resolving a symbolic link. /// /// Note also that there is no reentrant calling back to this method from within /// the node handling code. synchronized Node findNode(String name) { Node node = nodes.get(name); if (node == null) { - node = buildNode(name); + // We cannot be given the root paths ("/modules" or "/packages") + // because those nodes are already in the nodes cache. + if (name.startsWith(MODULES_ROOT + "/")) { + node = buildModulesNode(name); + } else if (name.startsWith(PACKAGES_ROOT + "/")) { + node = buildPackagesNode(name); + } if (node != null) { nodes.put(node.getName(), node); } @@ -291,44 +295,83 @@ synchronized Node findNode(String name) { return node; } - /// Builds a new, "complete" node based on its absolute name. - /// We only get here if the name is not yet present in the nodes map. - private Node buildNode(String name) { - // Zero-allocation test to reject any unexpected paths early. - boolean isPackages = name.startsWith("/packages"); - boolean isModules = !isPackages && name.startsWith("/modules"); - if (!(isModules || isPackages)) { - return null; - } - // Will FAIL for non-directory resources, since the image path does not + /// Builds a node in the "/modules/..." namespace. + /// + /// Called by `findNode()` if a `/modules` node is not present in the cache. + Node buildModulesNode(String name) { + assert name.startsWith(MODULES_ROOT + "/") : "Invalid module node name: " + name; + // Will fail for non-directory resources, since the image path does not // start with "/modules" (e.g. "/java.base/java/lang/Object.class"). ImageLocation loc = findLocation(name); - - Node node = null; if (loc != null) { - // A subtree node in either /modules/... or /packages/... - node = isPackages - ? buildPackageNode(name, loc) - : buildModuleDirectory(name, loc); - } else if (isModules) { - // We still cannot trust that this path is valid, but if it is, - // it must be a resource node in /modules/... - node = buildResourceNode(name); + assert name.equals(loc.getFullName()) : "Mismatched location for directory: " + name; + assert isModulesSubdirectory(loc) : "Invalid modules directory: " + name; + return completeModuleDirectory(newDirectory(name), loc); } - return node; + // TODO (review note): This is a heuristic to avoid spending time on lookup + // in cases of failure, but is not strictly required for correct behaviour. + // If there is no ImageLocation, the given name cannot be trusted to + // belong to any resource (or even be a valid resource name). + int moduleEnd = name.indexOf('/', MODULES_ROOT.length() + 1); + if (moduleEnd == -1) { + return null; + } + // Tests that the implied module name actually exists before doing + // a full lookup for the location. + ImageLocation moduleLoc = findLocation(name.substring(0, moduleEnd)); + if (moduleLoc == null || moduleLoc.getModuleOffset() == 0) { + return null; + } + // Resource paths in the image are NOT prefixed with "/modules". + ImageLocation resourceLoc = findLocation(name.substring(MODULES_ROOT.length())); + return resourceLoc != null ? newResource(name, resourceLoc) : null; + } + + /// Builds a node in the "/packages/..." namespace. + /// + /// Called by `findNode()` if a `/packages` node is not present in the cache. + private Node buildPackagesNode(String name) { + // There are only locations for the root "/packages" or "/packages/xxx" + // directories, but not the symbolic links below them (the links can be + // entirely derived from the name information in the parent directory). + // However, unlike resources this means that we do not have a constant + // time lookup for link nodes when creating them. + int packageStart = PACKAGES_ROOT.length() + 1; + int packageEnd = name.indexOf('/', packageStart); + if (packageEnd == -1) { + ImageLocation loc = findLocation(name); + return loc != null ? completePackageDirectory(newDirectory(name), loc) : null; + } else { + // We cannot assume that because the given name was not cached, the + // directory exists (the given name is untrusted and could reference + // a non-existent link). However, *if* the parent directory *is* + // present, we can conclude that the given name is not a valid link. + String dirName = name.substring(0, packageEnd); + if (!nodes.containsKey(dirName)) { + ImageLocation loc = findLocation(dirName); + if (loc != null) { + Directory dir = completePackageDirectory(newDirectory(dirName), loc); + // When the parent is created, its child nodes are cached. + nodes.put(dir.getName(), dir); + return nodes.get(name); + } + } + } + return null; } /// Completes a directory by ensuring its child list is populated correctly. private void completeDirectory(Directory dir) { String name = dir.getName(); - // Since the node exists, we can assert that it's name starts with - // either /modules or /packages, making differentiation easy. It also - // means that the name is valid and which must yield a location. - assert name.startsWith("/modules") || name.startsWith("/packages"); + // Since the node exists, we can assert that its name starts with + // either "/modules" or "/packages", making differentiation easy. It + // also means that the name is valid, so it must yield a location. + assert name.startsWith(MODULES_ROOT) || name.startsWith(PACKAGES_ROOT); ImageLocation loc = findLocation(name); assert loc != null && name.equals(loc.getFullName()) : "Invalid location for name: " + name; // We cannot use 'isXxxSubdirectory()' methods here since we could - // be given a top-level directory. + // be given a top-level directory (for which that test doesn't work). + // TODO (review note): I feel a bit dirty putting this test in, but it is fast and accurate. if (name.charAt(1) == 'm') { completeModuleDirectory(dir, loc); } else { @@ -337,36 +380,37 @@ private void completeDirectory(Directory dir) { assert dir.isCompleted() : "Directory must be complete by now: " + dir; } - /// Builds either a 2nd level package directory, or a 3rd level symbolic - /// link from an {@code ImageLocation}. Possible names are: - /// - /// * {@code /packages/}: Builds a {@code Directory}. - /// * {@code /packages//}: Builds a {@code LinkNode}. + /// Completes a modules directory by setting the list of child nodes. /// - /// Called by {@code buildNode()} if a package node is not present in the - /// cache. The top-level {@code /packages} directory was already built by - /// {@code buildRootDirectory}. - private Node buildPackageNode(String name, ImageLocation loc) { - assert !name.equals("/packages") : "Cannot build root /packages directory"; - if (isPackagesSubdirectory(loc)) { - return completePackageDirectory(newDirectory(name), loc); - } - int modStart = name.indexOf('/', "/packages/".length()) + 1; - assert modStart > 0 && name.indexOf('/', modStart) == -1 : "Invalid link name: " + name; - return newLinkNode(name, "/modules/" + name.substring(modStart)); + /// The given directory can be the top level `/modules` directory, so + /// it is NOT safe to use `isModulesSubdirectory(loc)` here. + private Directory completeModuleDirectory(Directory dir, ImageLocation loc) { + assert dir.getName().equals(loc.getFullName()) : "Mismatched location for directory: " + dir; + List children = createChildNodes(loc, (childloc) -> { + if (isModulesSubdirectory(childloc)) { + return nodes.computeIfAbsent(childloc.getFullName(), this::newDirectory); + } else { + // Add the "/modules" prefix to image location paths to get + // Jrt file system names. + String resourceName = childloc.getFullName(true); + return nodes.computeIfAbsent(resourceName, n -> newResource(n, childloc)); + } + }); + dir.setChildren(children); + return dir; } /// Completes a package directory by setting the list of child nodes. /// - /// This is called by {@code buildPackageNode()}, but also for the - /// top-level {@code /packages} directory. + /// The given directory can be the top level `/packages` directory, so + /// it is NOT safe to use `isPackagesSubdirectory(loc)` here. private Directory completePackageDirectory(Directory dir, ImageLocation loc) { assert dir.getName().equals(loc.getFullName()) : "Mismatched location for directory: " + dir; - // The only directories in the /packages namespace are /packages or - // /packages/. However, unlike /modules directories, the + // The only directories in the "/packages" namespace are "/packages" or + // "/packages/". However, unlike "/modules" directories, the // location offsets mean different things. List children; - if (dir.getName().equals("/packages")) { + if (dir.getName().equals(PACKAGES_ROOT)) { // Top-level directory just contains a list of subdirectories. children = createChildNodes(loc, c -> nodes.computeIfAbsent(c.getFullName(), this::newDirectory)); } else { @@ -388,70 +432,8 @@ private Directory completePackageDirectory(Directory dir, ImageLocation loc) { return dir; } - /// Builds a modules subdirectory directory from an {@code ImageLocation}. - /// - /// Called by {@code buildNode()} if a module directory is not present in - /// the cache. The top-level {@code /modules} directory was already built by - /// {@code buildRootDirectory}. - private Node buildModuleDirectory(String name, ImageLocation loc) { - assert name.equals(loc.getFullName()) : "Mismatched location for directory: " + name; - assert isModulesSubdirectory(loc) : "Invalid modules directory: " + name; - return completeModuleDirectory(newDirectory(name), loc); - } - - /// Completes a modules directory by setting the list of child nodes. - /// - /// The given directory can be the top level {@code /modules} directory - /// (so it is NOT safe to use {@code isModulesSubdirectory(loc)} here). - private Directory completeModuleDirectory(Directory dir, ImageLocation loc) { - assert dir.getName().equals(loc.getFullName()) : "Mismatched location for directory: " + dir; - List children = createChildNodes(loc, (childloc) -> { - if (isModulesSubdirectory(childloc)) { - return nodes.computeIfAbsent(childloc.getFullName(), this::newDirectory); - } else { - // Add the "/modules" prefix to image location paths to get - // Jrt file system names. - String resourceName = childloc.getFullName(true); - return nodes.computeIfAbsent(resourceName, n -> newResource(n, childloc)); - } - }); - dir.setChildren(children); - return dir; - } - - /// Builds a resource node in the {@code /modules} hierarchy. - /// - /// Called by {@code buildNode()} if there was no entry for the given - /// name in the cache. Unlike {@code buildPackageNode()} and - /// {@code buildModuleDirectory()}, there is no ImageLocation (yet) and - /// the given name cannot be trusted to belong to any resource (or even - /// be a valid resource name). - private Node buildResourceNode(String name) { - if (!name.startsWith("/modules/")) { - return null; - } - // Make sure that the thing that follows "/modules/" is a module name. - int moduleEndIndex = name.indexOf('/', "/modules/".length()); - if (moduleEndIndex == -1) { - return null; - } - // Tests that the implied module name actually exists before during - // a full lookup for the location. - // TODO: I don't think this is strictly necessary (maybe an assert)? - ImageLocation moduleLoc = findLocation(name.substring(0, moduleEndIndex)); - if (moduleLoc == null || moduleLoc.getModuleOffset() == 0) { - return null; - } - // Resource paths in the image are NOT prefixed with /modules. - ImageLocation resourceLoc = findLocation(name.substring("/modules".length())); - if (resourceLoc == null) { - return null; - } - return newResource(name, resourceLoc); - } - - /// Creates the list of child nodes for a {@code Directory} based on a - /// given node creation function. + /// Creates the list of child nodes for a `Directory` based on a given + /// node creation function. /// /// Note: This cannot be used for package subdirectories as they have /// child offsets stored differently to other directories. @@ -465,8 +447,7 @@ private List createChildNodes(ImageLocation loc, Function 0; + return loc.getModuleOffset() == packagesStringOffset; } /// Determines if an image location represents a directory of some kind. - /// Directory locations store child offsets in their content. private boolean isDirectory(ImageLocation loc) { - return isModulesSubdirectory(loc) || isPackagesSubdirectory(loc) - || loc.getFullName().equals("/modules") || loc.getFullName().equals("/packages"); + return isModulesSubdirectory(loc) || isPackagesSubdirectory(loc) || loc.getModuleOffset() == 0; } /// Returns the file attributes of the image file. Currently, all nodes @@ -519,14 +496,14 @@ private BasicFileAttributes imageFileAttributes() { /// Creates an "incomplete" directory node with no child nodes set. /// Directories need to be "completed" before they are returned by - /// {@code findNode()}. + /// `findNode()`. private Directory newDirectory(String name) { return new Directory(name, imageFileAttributes()); } /// Creates a new resource from an image location. This is the only case /// where the image location name does not match the requested node name. - /// In image files, resource locations are NOT prefixed by {@code /modules}. + /// In image files, resource locations are NOT prefixed by `/modules`. private Resource newResource(String name, ImageLocation loc) { assert name.equals(loc.getFullName(true)) : "Mismatched location for resource: " + name; return new Resource(name, loc, imageFileAttributes()); @@ -534,9 +511,8 @@ private Resource newResource(String name, ImageLocation loc) { /// Creates a new link node pointing at the given target name. /// - /// Note that target node is resolved each time {@code resolve()} is - /// called, so if a link node is retained after its reader is closed, - /// it will begin to fail. + /// Note that target node is resolved each time `resolve()` is called, so + /// if a link node is retained after its reader is closed, it will fail. private LinkNode newLinkNode(String name, String targetName) { return new LinkNode(name, () -> findNode(targetName), imageFileAttributes()); } @@ -551,6 +527,9 @@ private byte[] getResource(Node node) throws IOException { } } + /** + * A directory, resource or symbolic link in the JRT file system namespace. + */ public abstract static class Node { private final String name; private final BasicFileAttributes fileAttrs; @@ -567,77 +546,109 @@ boolean isCompleted() { return true; } + // Only resources can return a location. + ImageLocation getLocation() { + throw new IllegalArgumentException("not a resource: " + getName()); + } + + /** + * Returns the JRY file system compatible name of this node (e.g. + * {@code "/modules/java.base/java/lang/Object.class"} or {@code + * "/packages/java.lang"}). + */ public final String getName() { return name; } + /** + * Returns file attributes for this node. The value returned may be the + * same for all nodes, and should not be relied upon for accuracy. + */ public final BasicFileAttributes getFileAttributes() { return fileAttrs; } - // resolve this Node (if this is a soft link, get underlying Node) + /** + * Resolves a symbolic link to its target node. If this code is not a + * symbolic link, then it resolves to itself. + */ public final Node resolveLink() { return resolveLink(false); } + /** + * Resolves a symbolic link to its target node. If this code is not a + * symbolic link, then it resolves to itself. + */ public Node resolveLink(boolean recursive) { return this; } - // is this a soft link Node? + /** Returns whether this node is a symbolic link. */ public boolean isLink() { return false; } + /** + * Returns whether this node is a directory. Directory nodes can have + * {@link #getChildNames()} invoked to get the fully qualified names + * of any child nodes. + */ public boolean isDirectory() { return false; } - public Stream getChildNames() { - throw new IllegalArgumentException("not a directory: " + getNameString()); - } - + /** + * Returns whether this node is a resource. Resource nodes can have + * their contents obtained via {@link ImageReader#getResource(Node)} + * or {@link ImageReader#getResourceBuffer(Node)}. + */ public boolean isResource() { return false; } - public ImageLocation getLocation() { - throw new IllegalArgumentException("not a resource: " + getNameString()); + // TODO (review note): Sure this could/should be IllegalStateException? + /** + * Returns the fully qualified names of any child nodes for a directory. + * + *

If this node is not a directory ({@code isDirectory() == false}) + * then this method will throw {@link IllegalArgumentException}. + */ + public Stream getChildNames() { + throw new IllegalArgumentException("not a directory: " + getName()); } + /** + * Returns the uncompressed size of this node's content. If this node is + * not a resource, this method returns zero. + */ public long size() { return 0L; } + /** + * Returns the compressed size of this node's content. If this node is + * not a resource, this method returns zero. + */ public long compressedSize() { return 0L; } + /** + * Returns the extension string of a resource node. If this node is not + * a resource, this method returns null. + */ public String extension() { return null; } - public final FileTime creationTime() { - return fileAttrs.creationTime(); - } - - public final FileTime lastAccessTime() { - return fileAttrs.lastAccessTime(); - } - - public final FileTime lastModifiedTime() { - return fileAttrs.lastModifiedTime(); - } - - public final String getNameString() { - return name; - } - @Override public final String toString() { - return getNameString(); + return getName(); } + /// TODO (review note): In general, nodes are NOT comparable by name. They + /// can differ depending on the reader they came from, and soon preview mode. @Override public final int hashCode() { return name.hashCode(); @@ -667,14 +678,14 @@ public final boolean equals(Object other) { /// but this DOES NOT mean that its child nodes are complete yet. /// /// To avoid users being able to access incomplete child nodes, the - /// {@link Node} API offers only a way to obtain child node names, forcing + /// `Node` API offers only a way to obtain child node names, forcing /// callers to invoke {@link ImageReader#findNode(String)} if they need to /// access the child node itself. /// /// This approach allows directories to be implemented lazily with respect /// to child nodes, while retaining efficiency when child nodes are accessed - /// (since the incomplete node will be created and placed in the node cache - /// as the parent is accessed). + /// (since any incomplete nodes will be created and placed in the node cache + /// when the parent was first returned to the user). private static final class Directory extends Node { // Monotonic reference, will be set to the unmodifiable child list exactly once. private List children = null; @@ -707,7 +718,7 @@ private void setChildren(List children) { } } - /// Resource node (a ".class" or any other data resource in jimage). + /// Resource node (e.g. a ".class" entry, or any other data resource). /// /// Resources are leaf nodes referencing an underlying image location. They /// are lightweight, and do not cache their contents. @@ -726,13 +737,13 @@ private Resource(String name, ImageLocation loc, BasicFileAttributes fileAttrs) } @Override - public boolean isResource() { - return true; + ImageLocation getLocation() { + return loc; } @Override - public ImageLocation getLocation() { - return loc; + public boolean isResource() { + return true; } @Override diff --git a/src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java b/src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java index 2925dc2e273f0..87a00da4393ba 100644 --- a/src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java +++ b/src/java.base/share/classes/jdk/internal/jrtfs/ExplodedImage.java @@ -121,7 +121,7 @@ byte[] getContent() throws IOException { @Override public Stream getChildNames() { if (!isDirectory()) - throw new IllegalArgumentException("not a directory: " + getNameString()); + throw new IllegalArgumentException("not a directory: " + getName()); if (children == null) { List list = new ArrayList<>(); try (DirectoryStream stream = Files.newDirectoryStream(path)) { diff --git a/src/java.base/share/classes/jdk/internal/jrtfs/JrtFileAttributes.java b/src/java.base/share/classes/jdk/internal/jrtfs/JrtFileAttributes.java index f0804b58c1c41..ea588c05012f4 100644 --- a/src/java.base/share/classes/jdk/internal/jrtfs/JrtFileAttributes.java +++ b/src/java.base/share/classes/jdk/internal/jrtfs/JrtFileAttributes.java @@ -49,7 +49,7 @@ final class JrtFileAttributes implements BasicFileAttributes { //-------- basic attributes -------- @Override public FileTime creationTime() { - return node.creationTime(); + return node.getFileAttributes().creationTime(); } @Override @@ -69,12 +69,12 @@ public boolean isRegularFile() { @Override public FileTime lastAccessTime() { - return node.lastAccessTime(); + return node.getFileAttributes().lastAccessTime(); } @Override public FileTime lastModifiedTime() { - return node.lastModifiedTime(); + return node.getFileAttributes().lastModifiedTime(); } @Override diff --git a/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java b/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java index 92a895868f40d..be6c9c1dc4386 100644 --- a/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java +++ b/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java @@ -51,9 +51,9 @@ public class JavaRuntimeURLConnection extends URLConnection { // // It is important to note that all of this information comes from the given // URL's path part, and there's no requirement for there to be distinct rules - // about percent encoding. However, we choose to treat the module name is if - // it were a URL authority (since Java package/module names are historically - // strongly associated with internet domain names). + // about percent encoding, and it is likely that any differences between how + // module names and resource names are treated is unintentional. The rules + // about percent encoding may well be tightened up in the future. // // The module name is not percent-decoded, and can be empty. private final String module; @@ -65,7 +65,7 @@ public class JavaRuntimeURLConnection extends URLConnection { JavaRuntimeURLConnection(URL url) throws IOException { super(url); - // TODO: Reject module names which appear to be using percent encoding. + // TODO: Allow percent encoding in module names. // TODO: Consider rejecting URLs with fragments, queries or authority. String urlPath = url.getPath(); if (urlPath.isEmpty() || urlPath.charAt(0) != '/') { @@ -134,7 +134,8 @@ private static String percentDecode(String path) throws MalformedURLException { // Nothing to decode (overwhelmingly common case). return path; } - // TODO: Reject over-encoded paths, especially %2F (/) and %24 ($). + // TODO: Maybe reject over-encoded paths here to reduce obfuscation + // (especially %2F (/) and %24 ($), but probably just all ASCII). try { return ParseUtil.decode(path); } catch (IllegalArgumentException e) { diff --git a/test/jdk/jdk/internal/jimage/ImageReaderTest.java b/test/jdk/jdk/internal/jimage/ImageReaderTest.java new file mode 100644 index 0000000000000..e6cecc2478efe --- /dev/null +++ b/test/jdk/jdk/internal/jimage/ImageReaderTest.java @@ -0,0 +1,267 @@ +/* + * 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. + */ + +import jdk.internal.jimage.ImageReader; +import jdk.internal.jimage.ImageReader.Node; +import jdk.test.lib.compiler.InMemoryJavaCompiler; +import jdk.test.lib.util.JarBuilder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.opentest4j.TestSkippedException; +import tests.Helper; +import tests.JImageGenerator; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; + +/* + * @test + * @summary Tests for ImageReader. + * @modules java.base/jdk.internal.jimage + * jdk.jlink/jdk.tools.jimage + * @library /test/jdk/tools/lib + * /test/lib + * @build tests.* + * @run junit/othervm ImageReaderTest + */ + +/// Using PER_CLASS lifecycle means the (expensive) image file is only build once. +/// There is no mutable test instance state to worry about. +@TestInstance(PER_CLASS) +public class ImageReaderTest { + + private static final Map> IMAGE_ENTRIES = Map.of( + "modfoo", Arrays.asList( + "com.foo.Alpha", + "com.foo.Beta", + "com.foo.bar.Gamma"), + "modbar", Arrays.asList( + "com.bar.One", + "com.bar.Two")); + private final Path image = buildJImage(IMAGE_ENTRIES); + + @Test + public void testModuleDirectories() throws IOException { + try (ImageReader reader = ImageReader.open(image)) { + // Basic directory expectations. + assertDir(reader, "/"); + assertDir(reader, "/modules"); + assertDir(reader, "/modules/modfoo"); + assertDir(reader, "/modules/modbar"); + assertDir(reader, "/modules/modfoo/com"); + assertDir(reader, "/modules/modfoo/com/foo"); + assertDir(reader, "/modules/modfoo/com/foo/bar"); + + assertAbsent(reader, "/modules/"); + assertAbsent(reader, "/modules/unknown"); + assertAbsent(reader, "/modules/modbar/"); + assertAbsent(reader, "/modules/modbar//com"); + assertAbsent(reader, "/modules/modbar/com/"); + } + } + + @Test + public void testModuleResources() throws IOException { + try (ImageReader reader = ImageReader.open(image)) { + assertNode(reader, "/modules/modfoo/com/foo/Alpha.class"); + assertNode(reader, "/modules/modbar/com/bar/One.class"); + + ImageClassLoader loader = new ImageClassLoader(reader, IMAGE_ENTRIES.keySet()); + assertEquals("Class: com.foo.Alpha", loader.loadAndGetToString("modfoo", "com.foo.Alpha")); + assertEquals("Class: com.foo.Beta", loader.loadAndGetToString("modfoo", "com.foo.Beta")); + assertEquals("Class: com.foo.bar.Gamma", loader.loadAndGetToString("modfoo", "com.foo.bar.Gamma")); + assertEquals("Class: com.bar.One", loader.loadAndGetToString("modbar", "com.bar.One")); + } + } + + @Test + public void testPackageDirectories() throws IOException { + try (ImageReader reader = ImageReader.open(image)) { + Node root = assertDir(reader, "/packages"); + Set pkgNames = root.getChildNames().collect(Collectors.toSet()); + assertTrue(pkgNames.contains("/packages/com")); + assertTrue(pkgNames.contains("/packages/com.foo")); + assertTrue(pkgNames.contains("/packages/com.bar")); + + // Even though no classes exist directly in the "com" package, it still + // creates a directory with links back to all the modules which contain it. + Set comLinks = assertDir(reader, "/packages/com").getChildNames().collect(Collectors.toSet()); + assertTrue(comLinks.contains("/packages/com/modfoo")); + assertTrue(comLinks.contains("/packages/com/modbar")); + } + } + + @Test + public void testPackageLinks() throws IOException { + try (ImageReader reader = ImageReader.open(image)) { + Node moduleFoo = assertDir(reader, "/modules/modfoo"); + Node moduleBar = assertDir(reader, "/modules/modbar"); + assertSame(assertLink(reader, "/packages/com.foo/modfoo").resolveLink(), moduleFoo); + assertSame(assertLink(reader, "/packages/com.bar/modbar").resolveLink(), moduleBar); + } + } + + private static ImageReader.Node assertNode(ImageReader reader, String name) throws IOException { + ImageReader.Node node = reader.findNode(name); + assertNotNull(node, "Could not find node: " + name); + return node; + } + + private static ImageReader.Node assertDir(ImageReader reader, String name) throws IOException { + ImageReader.Node dir = assertNode(reader, name); + assertTrue(dir.isDirectory(), "Node was not a directory: " + name); + return dir; + } + + private static ImageReader.Node assertLink(ImageReader reader, String name) throws IOException { + ImageReader.Node link = assertNode(reader, name); + assertTrue(link.isLink(), "Node was not a symbolic link: " + name); + return link; + } + + private static void assertAbsent(ImageReader reader, String name) throws IOException { + assertNull(reader.findNode(name), "Should not be able to find node: " + name); + } + + /// Builds a jimage file with the specified class entries. The classes in the built + /// image can be loaded and executed to return their names via `toString()` to confirm + /// the correct bytes were returned. + public static Path buildJImage(Map> entries) { + Helper helper = getHelper(); + Path outDir = helper.createNewImageDir("test"); + JImageGenerator.JLinkTask jlink = JImageGenerator.getJLinkTask() + .modulePath(helper.defaultModulePath()) + .output(outDir); + + Path jarDir = helper.getJarDir(); + IMAGE_ENTRIES.forEach((module, classes) -> { + JarBuilder jar = new JarBuilder(jarDir.resolve(module + ".jar").toString()); + String moduleInfo = "module " + module + " {}"; + jar.addEntry("module-info.class", InMemoryJavaCompiler.compile("module-info", moduleInfo)); + + classes.forEach(fqn -> { + int lastDot = fqn.lastIndexOf('.'); + String pkg = fqn.substring(0, lastDot); + String cls = fqn.substring(lastDot + 1); + + String path = fqn.replace('.', '/') + ".class"; + String source = String.format( + """ + package %s; + public class %s { + public String toString() { + return "Class: %s"; + } + } + """, pkg, cls, fqn); + jar.addEntry(path, InMemoryJavaCompiler.compile(fqn, source)); + }); + try { + jar.build(); + } catch (IOException e) { + throw new RuntimeException(e); + } + jlink.addMods(module); + }); + return jlink.call().assertSuccess().resolve("lib", "modules"); + } + + /// Returns the helper for building JAR and jimage files. + private static Helper getHelper() { + try { + Helper helper = Helper.newHelper(); + if (helper == null) { + throw new TestSkippedException("Cannot create test helper (exploded image?)"); + } + return helper; + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + /// Loads and performs actions on classes stored in a given `ImageReader`. + private static class ImageClassLoader extends ClassLoader { + private final ImageReader reader; + private final Set testModules; + + private ImageClassLoader(ImageReader reader, Set testModules) { + this.reader = reader; + this.testModules = testModules; + } + + @FunctionalInterface + public interface ClassAction { + R call(Class cls) throws T; + } + + String loadAndGetToString(String module, String fqn) { + return loadAndCall(module, fqn, c -> c.getDeclaredConstructor().newInstance().toString()); + } + + R loadAndCall(String module, String fqn, ClassAction action) { + Class cls = findClass(module, fqn); + assertNotNull(cls, "Could not load class: " + module + "/" + fqn); + try { + return action.call(cls); + } catch (Exception e) { + fail("Class loading failed", e); + return null; + } + } + + @Override + protected Class findClass(String module, String fqn) { + assumeTrue(testModules.contains(module), "Can only load classes in modules: " + testModules); + String name = "/modules/" + module + "/" + fqn.replace('.', '/') + ".class"; + Class cls = findLoadedClass(fqn); + if (cls == null) { + try { + ImageReader.Node node = reader.findNode(name); + if (node != null && node.isResource()) { + byte[] classBytes = reader.getResource(node); + cls = defineClass(fqn, classBytes, 0, classBytes.length); + resolveClass(cls); + return cls; + } + } catch (IOException e) { + throw new RuntimeException(e); + } + } + return null; + } + } +} \ No newline at end of file diff --git a/test/jdk/jdk/internal/jimage/JImageReadTest.java b/test/jdk/jdk/internal/jimage/JImageReadTest.java index 157262e42d332..1f77b4cc324ad 100644 --- a/test/jdk/jdk/internal/jimage/JImageReadTest.java +++ b/test/jdk/jdk/internal/jimage/JImageReadTest.java @@ -25,7 +25,7 @@ * @test * @summary Unit test for libjimage JIMAGE_Open/Read/Close * @modules java.base/jdk.internal.jimage - * @run testng JImageReadTest + * @run testng/othervm JImageReadTest */ import java.io.File; @@ -339,7 +339,7 @@ static void test5_imageReaderEndianness() throws IOException { // Will be opened with native byte order. try (ImageReader nativeReader = ImageReader.open(imageFile)) { // Just ensure something works as expected. - Assert.assertNotNull(nativeReader.getRootDirectory()); + Assert.assertNotNull(nativeReader.findNode("/")); } catch (IOException expected) { Assert.fail("Reader should be openable with native byte order."); } diff --git a/test/jdk/tools/jimage/ImageReaderDuplicateChildNodesTest.java b/test/jdk/tools/jimage/ImageReaderDuplicateChildNodesTest.java index bec32bee0f813..8656a4a3d00f1 100644 --- a/test/jdk/tools/jimage/ImageReaderDuplicateChildNodesTest.java +++ b/test/jdk/tools/jimage/ImageReaderDuplicateChildNodesTest.java @@ -68,17 +68,17 @@ public static void main(final String[] args) throws Exception { + " in " + imagePath); } // now verify that the parent node which is a directory, doesn't have duplicate children - final List children = parent.getChildren(); - if (children == null || children.isEmpty()) { + final List childNames = parent.getChildNames().toList(); + if (childNames.isEmpty()) { throw new RuntimeException("ImageReader did not return any child resources under " + integersParentResource + " in " + imagePath); } final Set uniqueChildren = new HashSet<>(); - for (final ImageReader.Node child : children) { - final boolean unique = uniqueChildren.add(child); + for (final String childName : childNames) { + final boolean unique = uniqueChildren.add(reader.findNode(childName)); if (!unique) { throw new RuntimeException("ImageReader returned duplicate child resource " - + child + " under " + parent + " from image " + imagePath); + + childName + " under " + parent + " from image " + imagePath); } } } From 2f7e175c6edb5e17ee23ee2b6fbb0af3d8f80d8c Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Sat, 28 Jun 2025 01:08:01 +0200 Subject: [PATCH 04/11] Newline at EOF. --- test/jdk/jdk/internal/jimage/ImageReaderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jdk/jdk/internal/jimage/ImageReaderTest.java b/test/jdk/jdk/internal/jimage/ImageReaderTest.java index e6cecc2478efe..07a5bb4f68eb7 100644 --- a/test/jdk/jdk/internal/jimage/ImageReaderTest.java +++ b/test/jdk/jdk/internal/jimage/ImageReaderTest.java @@ -264,4 +264,4 @@ protected Class findClass(String module, String fqn) { return null; } } -} \ No newline at end of file +} From cef0a625b4dff4c205aa7582ea9af9d369d3492d Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Sat, 28 Jun 2025 17:25:17 +0200 Subject: [PATCH 05/11] syncing to main branch --- .../jrt/JavaRuntimeURLConnection.java | 144 +++++++++++------- test/jdk/sun/net/www/protocol/jrt/Basic.java | 23 +-- 2 files changed, 89 insertions(+), 78 deletions(-) diff --git a/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java b/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java index be6c9c1dc4386..95e7b5f24376a 100644 --- a/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java +++ b/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 2024, 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 @@ -31,10 +31,11 @@ import java.net.MalformedURLException; import java.net.URL; +import jdk.internal.jimage.ImageLocation; import jdk.internal.jimage.ImageReader; -import jdk.internal.jimage.ImageReader.Node; import jdk.internal.jimage.ImageReaderFactory; +import jdk.internal.loader.Resource; import sun.net.www.ParseUtil; import sun.net.www.URLConnection; @@ -44,79 +45,99 @@ */ public class JavaRuntimeURLConnection extends URLConnection { - // ImageReader to access resources in jimage (never null). - private static final ImageReader READER = ImageReaderFactory.getImageReader(); - - // The module and resource name in the URL ("jrt://"). - // - // It is important to note that all of this information comes from the given - // URL's path part, and there's no requirement for there to be distinct rules - // about percent encoding, and it is likely that any differences between how - // module names and resource names are treated is unintentional. The rules - // about percent encoding may well be tightened up in the future. - // - // The module name is not percent-decoded, and can be empty. + // ImageReader to access resources in jimage + private static final ImageReader reader = ImageReaderFactory.getImageReader(); + + // the module and resource name in the URL private final String module; - // The resource name permits UTF-8 percent encoding of non-ASCII characters. private final String name; - // The resource node (when connected). - private volatile Node resource; + // the Resource when connected + private volatile Resource resource; JavaRuntimeURLConnection(URL url) throws IOException { super(url); - // TODO: Allow percent encoding in module names. - // TODO: Consider rejecting URLs with fragments, queries or authority. - String urlPath = url.getPath(); - if (urlPath.isEmpty() || urlPath.charAt(0) != '/') { + String path = url.getPath(); + if (path.isEmpty() || path.charAt(0) != '/') throw new MalformedURLException(url + " missing path or /"); - } - int pathSep = urlPath.indexOf('/', 1); - if (pathSep == -1) { - // No trailing resource path. This can never "connect" or return a - // resource, but might be useful as a representation to pass around. - // The module name *can* be empty here (e.g. "jrt:/") but not null. - this.module = urlPath.substring(1); + if (path.length() == 1) { + this.module = null; this.name = null; } else { - this.module = urlPath.substring(1, pathSep); - this.name = percentDecode(urlPath.substring(pathSep + 1)); + int pos = path.indexOf('/', 1); + if (pos == -1) { + this.module = path.substring(1); + this.name = null; + } else { + this.module = path.substring(1, pos); + this.name = ParseUtil.decode(path.substring(pos + 1)); + } } } /** - * Finds and caches the resource node associated with this URL and marks the - * connection as "connected". + * Finds a resource in a module, returning {@code null} if the resource + * is not found. */ - private synchronized Node getResourceNode() throws IOException { - if (resource == null) { - if (name == null) { - throw new IOException("cannot connect to jrt:/" + module); + private static Resource findResource(String module, String name) { + if (reader != null) { + URL url = toJrtURL(module, name); + ImageLocation location = reader.findLocation(module, name); + if (location != null) { + return new Resource() { + @Override + public String getName() { + return name; + } + @Override + public URL getURL() { + return url; + } + @Override + public URL getCodeSourceURL() { + return toJrtURL(module); + } + @Override + public InputStream getInputStream() throws IOException { + byte[] resource = reader.getResource(location); + return new ByteArrayInputStream(resource); + } + @Override + public int getContentLength() { + long size = location.getUncompressedSize(); + return (size > Integer.MAX_VALUE) ? -1 : (int) size; + } + }; } - Node node = READER.findNode("/modules/" + module + "/" + name); - if (node == null || !node.isResource()) { - throw new IOException(module + "/" + name + " not found"); - } - this.resource = node; - super.connected = true; } - return resource; + return null; } @Override - public void connect() throws IOException { - getResourceNode(); + public synchronized void connect() throws IOException { + if (!connected) { + if (name == null) { + String s = (module == null) ? "" : module; + throw new IOException("cannot connect to jrt:/" + s); + } + resource = findResource(module, name); + if (resource == null) + throw new IOException(module + "/" + name + " not found"); + connected = true; + } } @Override public InputStream getInputStream() throws IOException { - return new ByteArrayInputStream(READER.getResource(getResourceNode())); + connect(); + return resource.getInputStream(); } @Override public long getContentLengthLong() { try { - return getResourceNode().size(); + connect(); + return resource.getContentLength(); } catch (IOException ioe) { return -1L; } @@ -128,18 +149,27 @@ public int getContentLength() { return len > Integer.MAX_VALUE ? -1 : (int)len; } - // Perform percent decoding of the resource name/path from the URL. - private static String percentDecode(String path) throws MalformedURLException { - if (path.indexOf('%') == -1) { - // Nothing to decode (overwhelmingly common case). - return path; + /** + * Returns a jrt URL for the given module and resource name. + */ + @SuppressWarnings("deprecation") + private static URL toJrtURL(String module, String name) { + try { + return new URL("jrt:/" + module + "/" + name); + } catch (MalformedURLException e) { + throw new InternalError(e); } - // TODO: Maybe reject over-encoded paths here to reduce obfuscation - // (especially %2F (/) and %24 ($), but probably just all ASCII). + } + + /** + * Returns a jrt URL for the given module. + */ + @SuppressWarnings("deprecation") + private static URL toJrtURL(String module) { try { - return ParseUtil.decode(path); - } catch (IllegalArgumentException e) { - throw new MalformedURLException(e.getMessage()); + return new URL("jrt:/" + module); + } catch (MalformedURLException e) { + throw new InternalError(e); } } } diff --git a/test/jdk/sun/net/www/protocol/jrt/Basic.java b/test/jdk/sun/net/www/protocol/jrt/Basic.java index 3778912c375e5..c5dc234910c90 100644 --- a/test/jdk/sun/net/www/protocol/jrt/Basic.java +++ b/test/jdk/sun/net/www/protocol/jrt/Basic.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2014, 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 @@ -28,6 +28,7 @@ */ import java.io.IOException; +import java.io.InputStream; import java.net.URL; import java.net.URLConnection; @@ -41,27 +42,7 @@ public class Basic { public Object[][] urls() { Object[][] data = { {"jrt:/java.base/java/lang/Object.class", true}, - // Valid resource with and without percent-encoding. - {"jrt:/java.base/java/lang/Runtime$Version.class", true}, - {"jrt:/java.base/java%2Flang%2FRuntime%24Version.class", true}, - // Unnecessary percent encoding (just Object again). - {"jrt:/java.base/%6a%61%76%61%2f%6c%61%6e%67%2f%4f%62%6a%65%63%74%2e%63%6c%61%73%73", true}, - // Query parameters and fragments are silently ignored. - {"jrt:/java.base/java/lang/Object.class?yes=no", true}, - {"jrt:/java.base/java/lang/Object.class#anchor", true}, - - // Missing resource (no such class). - {"jrt:/java.base/java/lang/NoSuchClass.class", false}, - // Missing resource (wrong module). {"jrt:/java.desktop/java/lang/Object.class", false}, - // Entries in jimage which don't reference resources. - {"jrt:/modules/java.base/java/lang", false}, - {"jrt:/packages/java.lang", false}, - // Invalid (incomplete/corrupt) URIs. - {"jrt:/java.base", false}, - {"jrt:/java.base/", false}, - // Cannot escape anything in the module name. - {"jrt:/java%2Ebase/java/lang/Object.class", false}, }; return data; } From e551c3e66f595906f90d38064df70fb66e7ca919 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Sat, 28 Jun 2025 17:27:27 +0200 Subject: [PATCH 06/11] undo dependent changes --- .../sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java | 2 +- test/jdk/sun/net/www/protocol/jrt/Basic.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java b/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java index 95e7b5f24376a..4e67c962a46e1 100644 --- a/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java +++ b/src/java.base/share/classes/sun/net/www/protocol/jrt/JavaRuntimeURLConnection.java @@ -70,7 +70,7 @@ public class JavaRuntimeURLConnection extends URLConnection { this.name = null; } else { this.module = path.substring(1, pos); - this.name = ParseUtil.decode(path.substring(pos + 1)); + this.name = ParseUtil.decode(path.substring(pos+1)); } } } diff --git a/test/jdk/sun/net/www/protocol/jrt/Basic.java b/test/jdk/sun/net/www/protocol/jrt/Basic.java index c5dc234910c90..0f0492c9b39dc 100644 --- a/test/jdk/sun/net/www/protocol/jrt/Basic.java +++ b/test/jdk/sun/net/www/protocol/jrt/Basic.java @@ -41,8 +41,8 @@ public class Basic { @DataProvider(name = "urls") public Object[][] urls() { Object[][] data = { - {"jrt:/java.base/java/lang/Object.class", true}, - {"jrt:/java.desktop/java/lang/Object.class", false}, + { "jrt:/java.base/java/lang/Object.class", true }, + { "jrt:/java.desktop/java/lang/Object.class", false }, }; return data; } From d8efaa09768c3e1115635efd9a5a949919ff85ca Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Tue, 1 Jul 2025 01:24:11 +0200 Subject: [PATCH 07/11] Integrating with ImageReaderBenchmark. --- make/test/BuildMicrobenchmark.gmk | 1 + .../internal/jrtfs/ImageReaderBenchmark.java | 1072 +++++++++++++++++ 2 files changed, 1073 insertions(+) create mode 100644 test/micro/org/openjdk/bench/jdk/internal/jrtfs/ImageReaderBenchmark.java diff --git a/make/test/BuildMicrobenchmark.gmk b/make/test/BuildMicrobenchmark.gmk index 347ca44d25f39..4946263ef4e91 100644 --- a/make/test/BuildMicrobenchmark.gmk +++ b/make/test/BuildMicrobenchmark.gmk @@ -92,6 +92,7 @@ $(eval $(call SetupJavaCompilation, BUILD_JDK_MICROBENCHMARK, \ --add-exports java.base/jdk.internal.classfile.impl=ALL-UNNAMED \ --add-exports java.base/jdk.internal.event=ALL-UNNAMED \ --add-exports java.base/jdk.internal.foreign=ALL-UNNAMED \ + --add-exports java.base/jdk.internal.jimage=ALL-UNNAMED \ --add-exports java.base/jdk.internal.misc=ALL-UNNAMED \ --add-exports java.base/jdk.internal.util=ALL-UNNAMED \ --add-exports java.base/jdk.internal.vm=ALL-UNNAMED \ diff --git a/test/micro/org/openjdk/bench/jdk/internal/jrtfs/ImageReaderBenchmark.java b/test/micro/org/openjdk/bench/jdk/internal/jrtfs/ImageReaderBenchmark.java new file mode 100644 index 0000000000000..a9236b693d185 --- /dev/null +++ b/test/micro/org/openjdk/bench/jdk/internal/jrtfs/ImageReaderBenchmark.java @@ -0,0 +1,1072 @@ +/* + * 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. + */ +package org.openjdk.bench.jdk.internal.jrtfs; + +import jdk.internal.jimage.ImageReader; +import jdk.internal.jimage.ImageReader.Node; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.TearDown; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +import java.io.IOException; +import java.nio.ByteOrder; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; + +/// Benchmarks for ImageReader. See individual benchmarks for details on what they +/// measure, and their potential applicability for real world conclusions. +@BenchmarkMode(Mode.AverageTime) +@Warmup(iterations = 5, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 5, timeUnit = TimeUnit.MILLISECONDS) +@State(Scope.Benchmark) +@OutputTimeUnit(TimeUnit.MILLISECONDS) +@Fork(value = 1, jvmArgs = {"--add-exports", "java.base/jdk.internal.jimage=ALL-UNNAMED"}) +public class ImageReaderBenchmark { + + private static final Path SYSTEM_IMAGE_FILE = Path.of(System.getProperty("java.home"), "lib", "modules"); + static { + if (!Files.exists(SYSTEM_IMAGE_FILE)) { + throw new IllegalStateException("Cannot locate jimage file for benchmark: " + SYSTEM_IMAGE_FILE); + } + } + + /// NOT annotated with `@State` since it needs to potentially be used as a + /// per-benchmark or a per-iteration state object. The subclasses provide + /// any lifetime annotations that are needed. + static class BaseState { + protected Path copiedImageFile; + protected ByteOrder byteOrder; + long count = 0; + + public void setUp() throws IOException { + copiedImageFile = Files.createTempFile("copied_jimage", ""); + byteOrder = ByteOrder.nativeOrder(); + Files.copy(SYSTEM_IMAGE_FILE, copiedImageFile, REPLACE_EXISTING); + } + + public void tearDown() throws IOException { + Files.deleteIfExists(copiedImageFile); + System.err.println("Result: " + count); + } + } + + @State(Scope.Benchmark) + public static class WarmStartWithImageReader extends BaseState { + ImageReader reader; + + @Setup(Level.Trial) + public void setUp() throws IOException { + super.setUp(); + reader = ImageReader.open(copiedImageFile, byteOrder); + } + + @TearDown(Level.Trial) + public void tearDown() throws IOException { + super.tearDown(); + } + } + + @State(Scope.Benchmark) + public static class ColdStart extends BaseState { + @Setup(Level.Iteration) + public void setUp() throws IOException { + super.setUp(); + } + + @TearDown(Level.Iteration) + public void tearDown() throws IOException { + super.tearDown(); + } + } + + @State(Scope.Benchmark) + public static class ColdStartWithImageReader extends BaseState { + ImageReader reader; + + @Setup(Level.Iteration) + public void setup() throws IOException { + super.setUp(); + reader = ImageReader.open(copiedImageFile, byteOrder); + } + + @TearDown(Level.Iteration) + public void tearDown() throws IOException { + reader.close(); + super.tearDown(); + } + } + + /// Benchmarks counting of all nodes in the system image *after* they have all + /// been visited at least once. Image nodes should be cached after first use, + /// so this benchmark should be fast and very stable. + @Benchmark + @BenchmarkMode(Mode.AverageTime) + public void warmCache_CountAllNodes(WarmStartWithImageReader state) throws IOException { + state.count = countAllNodes(state.reader, state.reader.findNode("/")); + } + + /// Benchmarks counting of all nodes in the system image from a "cold start". This + /// visits all nodes in depth-first order and counts them. + /// + /// This benchmark is not representative of any typical usage pattern, but can be + /// used for comparisons between versions of `ImageReader`. + @Benchmark + @BenchmarkMode(Mode.SingleShotTime) + public void coldStart_InitAndCount(ColdStart state) throws IOException { + try (var reader = ImageReader.open(state.copiedImageFile, state.byteOrder)) { + state.count = countAllNodes(reader, reader.findNode("/")); + } + } + + /// As above, but includes the time to initialize the `ImageReader`. + @Benchmark + @BenchmarkMode(Mode.SingleShotTime) + public void coldStart_CountOnly(ColdStartWithImageReader state) throws IOException { + state.count = countAllNodes(state.reader, state.reader.findNode("/")); + } + + /// Benchmarks the time taken to load the byte array contents of classes + /// representative of those loaded by javac to for the simplest `HelloWorld` + /// program. + /// + /// This benchmark is somewhat representative of the cost of class loading + /// during javac startup. It is useful for comparisons between versions of + /// `ImageReader`, but also to estimate a lower bound for any reduction or + /// increase in the real-world startup time of javac. + @Benchmark + @BenchmarkMode(Mode.SingleShotTime) + public void coldStart_LoadJavacInitClasses(Blackhole bh, ColdStart state) throws IOException { + int errors = 0; + try (var reader = ImageReader.open(state.copiedImageFile, state.byteOrder)) { + for (String path : INIT_CLASSES) { + // Path determination isn't perfect so there can be a few "misses" in here. + // Report the count of bad paths as the "result", which should be < 20 or so. + Node node = reader.findNode(path); + if (node != null) { + bh.consume(reader.getResource(node)); + } else { + errors += 1; + } + } + } + state.count = INIT_CLASSES.size(); + // Allow up to 2% missing classes before complaining. + if ((100 * errors) / INIT_CLASSES.size() >= 2) { + reportMissingClassesAndFail(state, errors); + } + } + + static long countAllNodes(ImageReader reader, Node node) { + long count = 1; + if (node.isDirectory()) { + count += node.getChildNames().mapToLong(n -> { + try { + return countAllNodes(reader, reader.findNode(n)); + } catch (IOException e) { + throw new RuntimeException(e); + } + }).sum(); + } + return count; + } + + // Run if the INIT_CLASSES list below is sufficiently out-of-date. + // DO NOT run this before the benchmark, as it will cache all the nodes! + private static void reportMissingClassesAndFail(ColdStart state, int errors) throws IOException { + List missing = new ArrayList<>(errors); + try (var reader = ImageReader.open(state.copiedImageFile, state.byteOrder)) { + for (String path : INIT_CLASSES) { + if (reader.findNode(path) == null) { + missing.add(path); + } + } + } + throw new IllegalStateException( + String.format( + "Too many missing classes (%d of %d) in the hardcoded benchmark list.\n" + + "Please regenerate it according to instructions in the source code.\n" + + "Missing classes:\n\t%s", + errors, INIT_CLASSES.size(), String.join("\n\t", missing))); + } + + // Created by running "java -verbose:class", throwing away anonymous inner + // classes and anything without a reliable name, and grouping by the stated + // source. It's not perfect, but it's representative. + // + // /bin/java -verbose:class HelloWorld 2>&1 \ + // | fgrep '[class,load]' | cut -d' ' -f2 \ + // | tr '.' '/' \ + // | egrep -v '\$[0-9$]' \ + // | fgrep -v 'HelloWorld' \ + // | fgrep -v '/META-INF/preview/' \ + // | while read f ; do echo "${f}.class" ; done \ + // > initclasses.txt + // + // Output: + // java/lang/Object.class + // java/io/Serializable.class + // ... + // + // jimage list /images/jdk/lib/modules \ + // | awk '/^Module: */ { MOD=$2 }; /^ */ { print "/modules/"MOD"/"$1 }' \ + // > fullpaths.txt + // + // Output: + // ... + // /modules/java.base/java/lang/Object.class + // /modules/java.base/java/lang/OutOfMemoryError.class + // ... + // + // while read c ; do grep "/$c" fullpaths.txt ; done < initclasses.txt \ + // | while read c ; do printf ' "%s",\n' "$c" ; done \ + // > initpaths.txt + // + // Output: + private static final Set INIT_CLASSES = Set.of( + "/modules/java.base/java/lang/Object.class", + "/modules/java.base/java/io/Serializable.class", + "/modules/java.base/java/lang/Comparable.class", + "/modules/java.base/java/lang/CharSequence.class", + "/modules/java.base/java/lang/constant/Constable.class", + "/modules/java.base/java/lang/constant/ConstantDesc.class", + "/modules/java.base/java/lang/String.class", + "/modules/java.base/java/lang/reflect/AnnotatedElement.class", + "/modules/java.base/java/lang/reflect/GenericDeclaration.class", + "/modules/java.base/java/lang/reflect/Type.class", + "/modules/java.base/java/lang/invoke/TypeDescriptor.class", + "/modules/java.base/java/lang/invoke/TypeDescriptor$OfField.class", + "/modules/java.base/java/lang/Class.class", + "/modules/java.base/java/lang/Cloneable.class", + "/modules/java.base/java/lang/ClassLoader.class", + "/modules/java.base/java/lang/System.class", + "/modules/java.base/java/lang/Throwable.class", + "/modules/java.base/java/lang/Error.class", + "/modules/java.base/java/lang/Exception.class", + "/modules/java.base/java/lang/RuntimeException.class", + "/modules/java.base/java/security/ProtectionDomain.class", + "/modules/java.base/java/security/SecureClassLoader.class", + "/modules/java.base/java/lang/ReflectiveOperationException.class", + "/modules/java.base/java/lang/ClassNotFoundException.class", + "/modules/java.base/java/lang/Record.class", + "/modules/java.base/java/lang/LinkageError.class", + "/modules/java.base/java/lang/NoClassDefFoundError.class", + "/modules/java.base/java/lang/ClassCastException.class", + "/modules/java.base/java/lang/ArrayStoreException.class", + "/modules/java.base/java/lang/VirtualMachineError.class", + "/modules/java.base/java/lang/InternalError.class", + "/modules/java.base/java/lang/OutOfMemoryError.class", + "/modules/java.base/java/lang/StackOverflowError.class", + "/modules/java.base/java/lang/IllegalMonitorStateException.class", + "/modules/java.base/java/lang/ref/Reference.class", + "/modules/java.base/java/lang/IllegalCallerException.class", + "/modules/java.base/java/lang/ref/SoftReference.class", + "/modules/java.base/java/lang/ref/WeakReference.class", + "/modules/java.base/java/lang/ref/FinalReference.class", + "/modules/java.base/java/lang/ref/PhantomReference.class", + "/modules/java.base/java/lang/ref/Finalizer.class", + "/modules/java.base/java/lang/Runnable.class", + "/modules/java.base/java/lang/Thread.class", + "/modules/java.base/java/lang/Thread$FieldHolder.class", + "/modules/java.base/java/lang/Thread$Constants.class", + "/modules/java.base/java/lang/Thread$UncaughtExceptionHandler.class", + "/modules/java.base/java/lang/ThreadGroup.class", + "/modules/java.base/java/lang/BaseVirtualThread.class", + "/modules/java.base/java/lang/VirtualThread.class", + "/modules/java.base/java/lang/ThreadBuilders$BoundVirtualThread.class", + "/modules/java.base/java/util/Map.class", + "/modules/java.base/java/util/Dictionary.class", + "/modules/java.base/java/util/Hashtable.class", + "/modules/java.base/java/util/Properties.class", + "/modules/java.base/java/lang/Module.class", + "/modules/java.base/java/lang/reflect/AccessibleObject.class", + "/modules/java.base/java/lang/reflect/Member.class", + "/modules/java.base/java/lang/reflect/Field.class", + "/modules/java.base/java/lang/reflect/Parameter.class", + "/modules/java.base/java/lang/reflect/Executable.class", + "/modules/java.base/java/lang/reflect/Method.class", + "/modules/java.base/java/lang/reflect/Constructor.class", + "/modules/java.base/jdk/internal/vm/ContinuationScope.class", + "/modules/java.base/jdk/internal/vm/Continuation.class", + "/modules/java.base/jdk/internal/vm/StackChunk.class", + "/modules/java.base/jdk/internal/reflect/MethodAccessor.class", + "/modules/java.base/jdk/internal/reflect/MethodAccessorImpl.class", + "/modules/java.base/jdk/internal/reflect/ConstantPool.class", + "/modules/java.base/java/lang/annotation/Annotation.class", + "/modules/java.base/jdk/internal/reflect/CallerSensitive.class", + "/modules/java.base/jdk/internal/reflect/ConstructorAccessor.class", + "/modules/java.base/jdk/internal/reflect/ConstructorAccessorImpl.class", + "/modules/java.base/jdk/internal/reflect/DirectConstructorHandleAccessor$NativeAccessor.class", + "/modules/java.base/java/lang/invoke/MethodHandle.class", + "/modules/java.base/java/lang/invoke/DirectMethodHandle.class", + "/modules/java.base/java/lang/invoke/VarHandle.class", + "/modules/java.base/java/lang/invoke/MemberName.class", + "/modules/java.base/java/lang/invoke/ResolvedMethodName.class", + "/modules/java.base/java/lang/invoke/MethodHandleNatives.class", + "/modules/java.base/java/lang/invoke/LambdaForm.class", + "/modules/java.base/java/lang/invoke/TypeDescriptor$OfMethod.class", + "/modules/java.base/java/lang/invoke/MethodType.class", + "/modules/java.base/java/lang/BootstrapMethodError.class", + "/modules/java.base/java/lang/invoke/CallSite.class", + "/modules/java.base/jdk/internal/foreign/abi/NativeEntryPoint.class", + "/modules/java.base/jdk/internal/foreign/abi/ABIDescriptor.class", + "/modules/java.base/jdk/internal/foreign/abi/VMStorage.class", + "/modules/java.base/jdk/internal/foreign/abi/UpcallLinker$CallRegs.class", + "/modules/java.base/java/lang/invoke/ConstantCallSite.class", + "/modules/java.base/java/lang/invoke/MutableCallSite.class", + "/modules/java.base/java/lang/invoke/VolatileCallSite.class", + "/modules/java.base/java/lang/AssertionStatusDirectives.class", + "/modules/java.base/java/lang/Appendable.class", + "/modules/java.base/java/lang/AbstractStringBuilder.class", + "/modules/java.base/java/lang/StringBuffer.class", + "/modules/java.base/java/lang/StringBuilder.class", + "/modules/java.base/jdk/internal/misc/UnsafeConstants.class", + "/modules/java.base/jdk/internal/misc/Unsafe.class", + "/modules/java.base/jdk/internal/module/Modules.class", + "/modules/java.base/java/lang/AutoCloseable.class", + "/modules/java.base/java/io/Closeable.class", + "/modules/java.base/java/io/InputStream.class", + "/modules/java.base/java/io/ByteArrayInputStream.class", + "/modules/java.base/java/net/URL.class", + "/modules/java.base/java/lang/Enum.class", + "/modules/java.base/java/util/jar/Manifest.class", + "/modules/java.base/jdk/internal/loader/BuiltinClassLoader.class", + "/modules/java.base/jdk/internal/loader/ClassLoaders.class", + "/modules/java.base/jdk/internal/loader/ClassLoaders$AppClassLoader.class", + "/modules/java.base/jdk/internal/loader/ClassLoaders$PlatformClassLoader.class", + "/modules/java.base/java/security/CodeSource.class", + "/modules/java.base/java/util/concurrent/ConcurrentMap.class", + "/modules/java.base/java/util/AbstractMap.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap.class", + "/modules/java.base/java/lang/Iterable.class", + "/modules/java.base/java/util/Collection.class", + "/modules/java.base/java/util/SequencedCollection.class", + "/modules/java.base/java/util/List.class", + "/modules/java.base/java/util/RandomAccess.class", + "/modules/java.base/java/util/AbstractCollection.class", + "/modules/java.base/java/util/AbstractList.class", + "/modules/java.base/java/util/ArrayList.class", + "/modules/java.base/java/lang/StackTraceElement.class", + "/modules/java.base/java/nio/Buffer.class", + "/modules/java.base/java/lang/StackWalker.class", + "/modules/java.base/java/lang/StackStreamFactory$AbstractStackWalker.class", + "/modules/java.base/java/lang/StackWalker$StackFrame.class", + "/modules/java.base/java/lang/ClassFrameInfo.class", + "/modules/java.base/java/lang/StackFrameInfo.class", + "/modules/java.base/java/lang/LiveStackFrame.class", + "/modules/java.base/java/lang/LiveStackFrameInfo.class", + "/modules/java.base/java/util/concurrent/locks/AbstractOwnableSynchronizer.class", + "/modules/java.base/java/lang/Boolean.class", + "/modules/java.base/java/lang/Character.class", + "/modules/java.base/java/lang/Number.class", + "/modules/java.base/java/lang/Float.class", + "/modules/java.base/java/lang/Double.class", + "/modules/java.base/java/lang/Byte.class", + "/modules/java.base/java/lang/Short.class", + "/modules/java.base/java/lang/Integer.class", + "/modules/java.base/java/lang/Long.class", + "/modules/java.base/java/lang/Void.class", + "/modules/java.base/java/util/Iterator.class", + "/modules/java.base/java/lang/reflect/RecordComponent.class", + "/modules/java.base/jdk/internal/vm/vector/VectorSupport.class", + "/modules/java.base/jdk/internal/vm/vector/VectorSupport$VectorPayload.class", + "/modules/java.base/jdk/internal/vm/vector/VectorSupport$Vector.class", + "/modules/java.base/jdk/internal/vm/vector/VectorSupport$VectorMask.class", + "/modules/java.base/jdk/internal/vm/vector/VectorSupport$VectorShuffle.class", + "/modules/java.base/jdk/internal/vm/FillerObject.class", + "/modules/java.base/java/lang/NullPointerException.class", + "/modules/java.base/java/lang/ArithmeticException.class", + "/modules/java.base/java/lang/IndexOutOfBoundsException.class", + "/modules/java.base/java/lang/ArrayIndexOutOfBoundsException.class", + "/modules/java.base/java/io/ObjectStreamField.class", + "/modules/java.base/java/util/Comparator.class", + "/modules/java.base/java/lang/String$CaseInsensitiveComparator.class", + "/modules/java.base/jdk/internal/misc/VM.class", + "/modules/java.base/java/lang/Module$ArchivedData.class", + "/modules/java.base/jdk/internal/misc/CDS.class", + "/modules/java.base/java/util/Set.class", + "/modules/java.base/java/util/ImmutableCollections$AbstractImmutableCollection.class", + "/modules/java.base/java/util/ImmutableCollections$AbstractImmutableSet.class", + "/modules/java.base/java/util/ImmutableCollections$Set12.class", + "/modules/java.base/java/util/Objects.class", + "/modules/java.base/java/util/ImmutableCollections.class", + "/modules/java.base/java/util/ImmutableCollections$AbstractImmutableList.class", + "/modules/java.base/java/util/ImmutableCollections$ListN.class", + "/modules/java.base/java/util/ImmutableCollections$SetN.class", + "/modules/java.base/java/util/ImmutableCollections$AbstractImmutableMap.class", + "/modules/java.base/java/util/ImmutableCollections$MapN.class", + "/modules/java.base/jdk/internal/access/JavaLangReflectAccess.class", + "/modules/java.base/java/lang/reflect/ReflectAccess.class", + "/modules/java.base/jdk/internal/access/SharedSecrets.class", + "/modules/java.base/jdk/internal/reflect/ReflectionFactory.class", + "/modules/java.base/java/io/ObjectStreamClass.class", + "/modules/java.base/java/lang/Math.class", + "/modules/java.base/jdk/internal/reflect/ReflectionFactory$Config.class", + "/modules/java.base/jdk/internal/access/JavaLangRefAccess.class", + "/modules/java.base/java/lang/ref/ReferenceQueue.class", + "/modules/java.base/java/lang/ref/ReferenceQueue$Null.class", + "/modules/java.base/java/lang/ref/ReferenceQueue$Lock.class", + "/modules/java.base/jdk/internal/access/JavaLangAccess.class", + "/modules/java.base/jdk/internal/util/SystemProps.class", + "/modules/java.base/jdk/internal/util/SystemProps$Raw.class", + "/modules/java.base/java/nio/charset/Charset.class", + "/modules/java.base/java/nio/charset/spi/CharsetProvider.class", + "/modules/java.base/sun/nio/cs/StandardCharsets.class", + "/modules/java.base/java/lang/StringLatin1.class", + "/modules/java.base/sun/nio/cs/HistoricallyNamedCharset.class", + "/modules/java.base/sun/nio/cs/Unicode.class", + "/modules/java.base/sun/nio/cs/UTF_8.class", + "/modules/java.base/java/util/HashMap.class", + "/modules/java.base/java/lang/StrictMath.class", + "/modules/java.base/jdk/internal/util/ArraysSupport.class", + "/modules/java.base/java/util/Map$Entry.class", + "/modules/java.base/java/util/HashMap$Node.class", + "/modules/java.base/java/util/LinkedHashMap$Entry.class", + "/modules/java.base/java/util/HashMap$TreeNode.class", + "/modules/java.base/java/lang/StringConcatHelper.class", + "/modules/java.base/java/lang/VersionProps.class", + "/modules/java.base/java/lang/Runtime.class", + "/modules/java.base/java/util/concurrent/locks/Lock.class", + "/modules/java.base/java/util/concurrent/locks/ReentrantLock.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap$Segment.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap$CounterCell.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap$Node.class", + "/modules/java.base/java/util/concurrent/locks/LockSupport.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap$ReservationNode.class", + "/modules/java.base/java/util/AbstractSet.class", + "/modules/java.base/java/util/HashMap$EntrySet.class", + "/modules/java.base/java/util/HashMap$HashIterator.class", + "/modules/java.base/java/util/HashMap$EntryIterator.class", + "/modules/java.base/jdk/internal/util/StaticProperty.class", + "/modules/java.base/java/io/FileInputStream.class", + "/modules/java.base/java/lang/System$In.class", + "/modules/java.base/java/io/FileDescriptor.class", + "/modules/java.base/jdk/internal/access/JavaIOFileDescriptorAccess.class", + "/modules/java.base/java/io/Flushable.class", + "/modules/java.base/java/io/OutputStream.class", + "/modules/java.base/java/io/FileOutputStream.class", + "/modules/java.base/java/lang/System$Out.class", + "/modules/java.base/java/io/FilterInputStream.class", + "/modules/java.base/java/io/BufferedInputStream.class", + "/modules/java.base/java/io/FilterOutputStream.class", + "/modules/java.base/java/io/PrintStream.class", + "/modules/java.base/java/io/BufferedOutputStream.class", + "/modules/java.base/java/io/Writer.class", + "/modules/java.base/java/io/OutputStreamWriter.class", + "/modules/java.base/sun/nio/cs/StreamEncoder.class", + "/modules/java.base/java/nio/charset/CharsetEncoder.class", + "/modules/java.base/sun/nio/cs/UTF_8$Encoder.class", + "/modules/java.base/java/nio/charset/CodingErrorAction.class", + "/modules/java.base/java/util/Arrays.class", + "/modules/java.base/java/nio/ByteBuffer.class", + "/modules/java.base/jdk/internal/misc/ScopedMemoryAccess.class", + "/modules/java.base/java/util/function/Function.class", + "/modules/java.base/jdk/internal/util/Preconditions.class", + "/modules/java.base/java/util/function/BiFunction.class", + "/modules/java.base/jdk/internal/access/JavaNioAccess.class", + "/modules/java.base/java/nio/HeapByteBuffer.class", + "/modules/java.base/java/nio/ByteOrder.class", + "/modules/java.base/java/io/BufferedWriter.class", + "/modules/java.base/java/lang/Terminator.class", + "/modules/java.base/jdk/internal/misc/Signal$Handler.class", + "/modules/java.base/jdk/internal/misc/Signal.class", + "/modules/java.base/java/util/Hashtable$Entry.class", + "/modules/java.base/jdk/internal/misc/Signal$NativeHandler.class", + "/modules/java.base/java/lang/Integer$IntegerCache.class", + "/modules/java.base/jdk/internal/misc/OSEnvironment.class", + "/modules/java.base/java/lang/Thread$State.class", + "/modules/java.base/java/lang/ref/Reference$ReferenceHandler.class", + "/modules/java.base/java/lang/Thread$ThreadIdentifiers.class", + "/modules/java.base/java/lang/ref/Finalizer$FinalizerThread.class", + "/modules/java.base/jdk/internal/ref/Cleaner.class", + "/modules/java.base/java/util/Collections.class", + "/modules/java.base/java/util/Collections$EmptySet.class", + "/modules/java.base/java/util/Collections$EmptyList.class", + "/modules/java.base/java/util/Collections$EmptyMap.class", + "/modules/java.base/java/lang/IllegalArgumentException.class", + "/modules/java.base/java/lang/invoke/MethodHandleStatics.class", + "/modules/java.base/java/lang/reflect/ClassFileFormatVersion.class", + "/modules/java.base/java/lang/CharacterData.class", + "/modules/java.base/java/lang/CharacterDataLatin1.class", + "/modules/java.base/jdk/internal/util/ClassFileDumper.class", + "/modules/java.base/java/util/HexFormat.class", + "/modules/java.base/java/lang/Character$CharacterCache.class", + "/modules/java.base/java/util/concurrent/atomic/AtomicInteger.class", + "/modules/java.base/jdk/internal/module/ModuleBootstrap.class", + "/modules/java.base/java/lang/module/ModuleDescriptor.class", + "/modules/java.base/java/lang/invoke/MethodHandles.class", + "/modules/java.base/java/lang/invoke/MemberName$Factory.class", + "/modules/java.base/jdk/internal/reflect/Reflection.class", + "/modules/java.base/java/lang/invoke/MethodHandles$Lookup.class", + "/modules/java.base/java/util/ImmutableCollections$MapN$MapNIterator.class", + "/modules/java.base/java/util/KeyValueHolder.class", + "/modules/java.base/sun/invoke/util/VerifyAccess.class", + "/modules/java.base/java/lang/reflect/Modifier.class", + "/modules/java.base/jdk/internal/access/JavaLangModuleAccess.class", + "/modules/java.base/java/io/File.class", + "/modules/java.base/java/io/DefaultFileSystem.class", + "/modules/java.base/java/io/FileSystem.class", + "/modules/java.base/java/io/UnixFileSystem.class", + "/modules/java.base/jdk/internal/util/DecimalDigits.class", + "/modules/java.base/jdk/internal/module/ModulePatcher.class", + "/modules/java.base/jdk/internal/module/ModuleBootstrap$IllegalNativeAccess.class", + "/modules/java.base/java/util/HashSet.class", + "/modules/java.base/jdk/internal/module/ModuleLoaderMap.class", + "/modules/java.base/jdk/internal/module/ModuleLoaderMap$Modules.class", + "/modules/java.base/jdk/internal/module/ModuleBootstrap$Counters.class", + "/modules/java.base/jdk/internal/module/ArchivedBootLayer.class", + "/modules/java.base/jdk/internal/module/ArchivedModuleGraph.class", + "/modules/java.base/jdk/internal/module/SystemModuleFinders.class", + "/modules/java.base/java/net/URI.class", + "/modules/java.base/jdk/internal/access/JavaNetUriAccess.class", + "/modules/java.base/jdk/internal/module/SystemModulesMap.class", + "/modules/java.base/jdk/internal/module/SystemModules.class", + "/modules/java.base/jdk/internal/module/ExplodedSystemModules.class", + "/modules/java.base/java/nio/file/Watchable.class", + "/modules/java.base/java/nio/file/Path.class", + "/modules/java.base/java/nio/file/FileSystems.class", + "/modules/java.base/sun/nio/fs/DefaultFileSystemProvider.class", + "/modules/java.base/java/nio/file/spi/FileSystemProvider.class", + "/modules/java.base/sun/nio/fs/AbstractFileSystemProvider.class", + "/modules/java.base/sun/nio/fs/UnixFileSystemProvider.class", + "/modules/java.base/sun/nio/fs/LinuxFileSystemProvider.class", + "/modules/java.base/java/nio/file/OpenOption.class", + "/modules/java.base/java/nio/file/StandardOpenOption.class", + "/modules/java.base/java/nio/file/FileSystem.class", + "/modules/java.base/sun/nio/fs/UnixFileSystem.class", + "/modules/java.base/sun/nio/fs/LinuxFileSystem.class", + "/modules/java.base/sun/nio/fs/UnixPath.class", + "/modules/java.base/sun/nio/fs/Util.class", + "/modules/java.base/java/lang/StringCoding.class", + "/modules/java.base/sun/nio/fs/UnixNativeDispatcher.class", + "/modules/java.base/jdk/internal/loader/BootLoader.class", + "/modules/java.base/java/lang/Module$EnableNativeAccess.class", + "/modules/java.base/jdk/internal/loader/NativeLibraries.class", + "/modules/java.base/jdk/internal/loader/ClassLoaderHelper.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap$CollectionView.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap$KeySetView.class", + "/modules/java.base/jdk/internal/loader/NativeLibraries$LibraryPaths.class", + "/modules/java.base/java/io/File$PathStatus.class", + "/modules/java.base/jdk/internal/loader/NativeLibraries$CountedLock.class", + "/modules/java.base/java/util/concurrent/locks/AbstractQueuedSynchronizer.class", + "/modules/java.base/java/util/concurrent/locks/ReentrantLock$Sync.class", + "/modules/java.base/java/util/concurrent/locks/ReentrantLock$NonfairSync.class", + "/modules/java.base/jdk/internal/loader/NativeLibraries$NativeLibraryContext.class", + "/modules/java.base/java/util/Queue.class", + "/modules/java.base/java/util/Deque.class", + "/modules/java.base/java/util/ArrayDeque.class", + "/modules/java.base/java/util/ArrayDeque$DeqIterator.class", + "/modules/java.base/jdk/internal/loader/NativeLibrary.class", + "/modules/java.base/jdk/internal/loader/NativeLibraries$NativeLibraryImpl.class", + "/modules/java.base/java/security/cert/Certificate.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap$ValuesView.class", + "/modules/java.base/java/util/Enumeration.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap$Traverser.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap$BaseIterator.class", + "/modules/java.base/java/util/concurrent/ConcurrentHashMap$ValueIterator.class", + "/modules/java.base/java/nio/file/attribute/BasicFileAttributes.class", + "/modules/java.base/java/nio/file/attribute/PosixFileAttributes.class", + "/modules/java.base/sun/nio/fs/UnixFileAttributes.class", + "/modules/java.base/sun/nio/fs/UnixFileStoreAttributes.class", + "/modules/java.base/sun/nio/fs/UnixMountEntry.class", + "/modules/java.base/java/nio/file/CopyOption.class", + "/modules/java.base/java/nio/file/LinkOption.class", + "/modules/java.base/java/nio/file/Files.class", + "/modules/java.base/sun/nio/fs/NativeBuffers.class", + "/modules/java.base/java/lang/ThreadLocal.class", + "/modules/java.base/jdk/internal/misc/CarrierThreadLocal.class", + "/modules/java.base/jdk/internal/misc/TerminatingThreadLocal.class", + "/modules/java.base/java/lang/ThreadLocal$ThreadLocalMap.class", + "/modules/java.base/java/lang/ThreadLocal$ThreadLocalMap$Entry.class", + "/modules/java.base/java/util/IdentityHashMap.class", + "/modules/java.base/java/util/Collections$SetFromMap.class", + "/modules/java.base/java/util/IdentityHashMap$KeySet.class", + "/modules/java.base/sun/nio/fs/NativeBuffer.class", + "/modules/java.base/jdk/internal/ref/CleanerFactory.class", + "/modules/java.base/java/util/concurrent/ThreadFactory.class", + "/modules/java.base/java/lang/ref/Cleaner.class", + "/modules/java.base/jdk/internal/ref/CleanerImpl.class", + "/modules/java.base/jdk/internal/ref/CleanerImpl$CleanableList.class", + "/modules/java.base/jdk/internal/ref/CleanerImpl$CleanableList$Node.class", + "/modules/java.base/java/lang/ref/Cleaner$Cleanable.class", + "/modules/java.base/jdk/internal/ref/PhantomCleanable.class", + "/modules/java.base/jdk/internal/ref/CleanerImpl$CleanerCleanable.class", + "/modules/java.base/jdk/internal/misc/InnocuousThread.class", + "/modules/java.base/sun/nio/fs/NativeBuffer$Deallocator.class", + "/modules/java.base/jdk/internal/ref/CleanerImpl$PhantomCleanableRef.class", + "/modules/java.base/java/lang/module/ModuleFinder.class", + "/modules/java.base/jdk/internal/module/ModulePath.class", + "/modules/java.base/java/util/jar/Attributes$Name.class", + "/modules/java.base/java/lang/reflect/Array.class", + "/modules/java.base/jdk/internal/perf/PerfCounter.class", + "/modules/java.base/jdk/internal/perf/Perf.class", + "/modules/java.base/sun/nio/ch/DirectBuffer.class", + "/modules/java.base/java/nio/MappedByteBuffer.class", + "/modules/java.base/java/nio/DirectByteBuffer.class", + "/modules/java.base/java/nio/Bits.class", + "/modules/java.base/java/util/concurrent/atomic/AtomicLong.class", + "/modules/java.base/jdk/internal/misc/VM$BufferPool.class", + "/modules/java.base/java/nio/LongBuffer.class", + "/modules/java.base/java/nio/DirectLongBufferU.class", + "/modules/java.base/java/util/zip/ZipConstants.class", + "/modules/java.base/java/util/zip/ZipFile.class", + "/modules/java.base/java/util/jar/JarFile.class", + "/modules/java.base/java/util/BitSet.class", + "/modules/java.base/jdk/internal/access/JavaUtilZipFileAccess.class", + "/modules/java.base/jdk/internal/access/JavaUtilJarAccess.class", + "/modules/java.base/java/util/jar/JavaUtilJarAccessImpl.class", + "/modules/java.base/java/lang/Runtime$Version.class", + "/modules/java.base/java/util/ImmutableCollections$List12.class", + "/modules/java.base/java/util/Optional.class", + "/modules/java.base/java/nio/file/attribute/DosFileAttributes.class", + "/modules/java.base/java/nio/file/attribute/AttributeView.class", + "/modules/java.base/java/nio/file/attribute/FileAttributeView.class", + "/modules/java.base/java/nio/file/attribute/BasicFileAttributeView.class", + "/modules/java.base/java/nio/file/attribute/DosFileAttributeView.class", + "/modules/java.base/java/nio/file/attribute/UserDefinedFileAttributeView.class", + "/modules/java.base/sun/nio/fs/UnixFileAttributeViews.class", + "/modules/java.base/sun/nio/fs/DynamicFileAttributeView.class", + "/modules/java.base/sun/nio/fs/AbstractBasicFileAttributeView.class", + "/modules/java.base/sun/nio/fs/UnixFileAttributeViews$Basic.class", + "/modules/java.base/sun/nio/fs/UnixFileAttributes$UnixAsBasicFileAttributes.class", + "/modules/java.base/java/nio/file/DirectoryStream$Filter.class", + "/modules/java.base/java/nio/file/Files$AcceptAllFilter.class", + "/modules/java.base/java/nio/file/DirectoryStream.class", + "/modules/java.base/java/nio/file/SecureDirectoryStream.class", + "/modules/java.base/sun/nio/fs/UnixSecureDirectoryStream.class", + "/modules/java.base/sun/nio/fs/UnixDirectoryStream.class", + "/modules/java.base/java/util/concurrent/locks/ReadWriteLock.class", + "/modules/java.base/java/util/concurrent/locks/ReentrantReadWriteLock.class", + "/modules/java.base/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.class", + "/modules/java.base/java/util/concurrent/locks/ReentrantReadWriteLock$Sync.class", + "/modules/java.base/java/util/concurrent/locks/ReentrantReadWriteLock$FairSync.class", + "/modules/java.base/java/util/concurrent/locks/ReentrantReadWriteLock$Sync$ThreadLocalHoldCounter.class", + "/modules/java.base/java/util/concurrent/locks/ReentrantReadWriteLock$ReadLock.class", + "/modules/java.base/java/util/concurrent/locks/ReentrantReadWriteLock$WriteLock.class", + "/modules/java.base/sun/nio/fs/UnixDirectoryStream$UnixDirectoryIterator.class", + "/modules/java.base/java/nio/file/attribute/FileAttribute.class", + "/modules/java.base/sun/nio/fs/UnixFileModeAttribute.class", + "/modules/java.base/sun/nio/fs/UnixChannelFactory.class", + "/modules/java.base/sun/nio/fs/UnixChannelFactory$Flags.class", + "/modules/java.base/java/util/Collections$EmptyIterator.class", + "/modules/java.base/java/nio/channels/Channel.class", + "/modules/java.base/java/nio/channels/ReadableByteChannel.class", + "/modules/java.base/java/nio/channels/WritableByteChannel.class", + "/modules/java.base/java/nio/channels/ByteChannel.class", + "/modules/java.base/java/nio/channels/SeekableByteChannel.class", + "/modules/java.base/java/nio/channels/GatheringByteChannel.class", + "/modules/java.base/java/nio/channels/ScatteringByteChannel.class", + "/modules/java.base/java/nio/channels/InterruptibleChannel.class", + "/modules/java.base/java/nio/channels/spi/AbstractInterruptibleChannel.class", + "/modules/java.base/java/nio/channels/FileChannel.class", + "/modules/java.base/sun/nio/ch/FileChannelImpl.class", + "/modules/java.base/sun/nio/ch/NativeDispatcher.class", + "/modules/java.base/sun/nio/ch/FileDispatcher.class", + "/modules/java.base/sun/nio/ch/UnixFileDispatcherImpl.class", + "/modules/java.base/sun/nio/ch/FileDispatcherImpl.class", + "/modules/java.base/sun/nio/ch/IOUtil.class", + "/modules/java.base/sun/nio/ch/Interruptible.class", + "/modules/java.base/sun/nio/ch/NativeThreadSet.class", + "/modules/java.base/sun/nio/ch/FileChannelImpl$Closer.class", + "/modules/java.base/java/nio/channels/Channels.class", + "/modules/java.base/sun/nio/ch/Streams.class", + "/modules/java.base/sun/nio/ch/SelChImpl.class", + "/modules/java.base/java/nio/channels/NetworkChannel.class", + "/modules/java.base/java/nio/channels/SelectableChannel.class", + "/modules/java.base/java/nio/channels/spi/AbstractSelectableChannel.class", + "/modules/java.base/java/nio/channels/SocketChannel.class", + "/modules/java.base/sun/nio/ch/SocketChannelImpl.class", + "/modules/java.base/sun/nio/ch/ChannelInputStream.class", + "/modules/java.base/java/lang/invoke/LambdaMetafactory.class", + "/modules/java.base/java/util/function/Supplier.class", + "/modules/java.base/jdk/internal/util/ReferencedKeySet.class", + "/modules/java.base/jdk/internal/util/ReferencedKeyMap.class", + "/modules/java.base/jdk/internal/util/ReferenceKey.class", + "/modules/java.base/jdk/internal/util/StrongReferenceKey.class", + "/modules/java.base/java/lang/invoke/MethodTypeForm.class", + "/modules/java.base/jdk/internal/util/WeakReferenceKey.class", + "/modules/java.base/sun/invoke/util/Wrapper.class", + "/modules/java.base/sun/invoke/util/Wrapper$Format.class", + "/modules/java.base/java/lang/constant/ConstantDescs.class", + "/modules/java.base/java/lang/constant/ClassDesc.class", + "/modules/java.base/jdk/internal/constant/ClassOrInterfaceDescImpl.class", + "/modules/java.base/jdk/internal/constant/ArrayClassDescImpl.class", + "/modules/java.base/jdk/internal/constant/ConstantUtils.class", + "/modules/java.base/java/lang/constant/DirectMethodHandleDesc$Kind.class", + "/modules/java.base/java/lang/constant/MethodTypeDesc.class", + "/modules/java.base/jdk/internal/constant/MethodTypeDescImpl.class", + "/modules/java.base/java/lang/constant/MethodHandleDesc.class", + "/modules/java.base/java/lang/constant/DirectMethodHandleDesc.class", + "/modules/java.base/jdk/internal/constant/DirectMethodHandleDescImpl.class", + "/modules/java.base/java/lang/constant/DynamicConstantDesc.class", + "/modules/java.base/jdk/internal/constant/PrimitiveClassDescImpl.class", + "/modules/java.base/java/lang/constant/DynamicConstantDesc$AnonymousDynamicConstantDesc.class", + "/modules/java.base/java/lang/invoke/LambdaForm$NamedFunction.class", + "/modules/java.base/java/lang/invoke/DirectMethodHandle$Holder.class", + "/modules/java.base/sun/invoke/util/ValueConversions.class", + "/modules/java.base/java/lang/invoke/MethodHandleImpl.class", + "/modules/java.base/java/lang/invoke/Invokers.class", + "/modules/java.base/java/lang/invoke/LambdaForm$Kind.class", + "/modules/java.base/java/lang/NoSuchMethodException.class", + "/modules/java.base/java/lang/invoke/LambdaForm$BasicType.class", + "/modules/java.base/java/lang/classfile/TypeKind.class", + "/modules/java.base/java/lang/invoke/LambdaForm$Name.class", + "/modules/java.base/java/lang/invoke/LambdaForm$Holder.class", + "/modules/java.base/java/lang/invoke/InvokerBytecodeGenerator.class", + "/modules/java.base/java/lang/classfile/AnnotationElement.class", + "/modules/java.base/java/lang/classfile/Annotation.class", + "/modules/java.base/java/lang/classfile/constantpool/ConstantPool.class", + "/modules/java.base/java/lang/classfile/constantpool/ConstantPoolBuilder.class", + "/modules/java.base/jdk/internal/classfile/impl/TemporaryConstantPool.class", + "/modules/java.base/java/lang/classfile/constantpool/PoolEntry.class", + "/modules/java.base/java/lang/classfile/constantpool/AnnotationConstantValueEntry.class", + "/modules/java.base/java/lang/classfile/constantpool/Utf8Entry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$Utf8EntryImpl.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$Utf8EntryImpl$State.class", + "/modules/java.base/jdk/internal/classfile/impl/AnnotationImpl.class", + "/modules/java.base/java/lang/classfile/ClassFileElement.class", + "/modules/java.base/java/lang/classfile/Attribute.class", + "/modules/java.base/java/lang/classfile/ClassElement.class", + "/modules/java.base/java/lang/classfile/MethodElement.class", + "/modules/java.base/java/lang/classfile/FieldElement.class", + "/modules/java.base/java/lang/classfile/attribute/RuntimeVisibleAnnotationsAttribute.class", + "/modules/java.base/jdk/internal/classfile/impl/Util$Writable.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractElement.class", + "/modules/java.base/jdk/internal/classfile/impl/UnboundAttribute.class", + "/modules/java.base/jdk/internal/classfile/impl/UnboundAttribute$UnboundRuntimeVisibleAnnotationsAttribute.class", + "/modules/java.base/java/lang/classfile/Attributes.class", + "/modules/java.base/java/lang/classfile/AttributeMapper.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractAttributeMapper.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractAttributeMapper$RuntimeVisibleAnnotationsMapper.class", + "/modules/java.base/java/lang/classfile/AttributeMapper$AttributeStability.class", + "/modules/java.base/java/lang/invoke/MethodHandleImpl$Intrinsic.class", + "/modules/java.base/jdk/internal/classfile/impl/SplitConstantPool.class", + "/modules/java.base/java/lang/classfile/BootstrapMethodEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/BootstrapMethodEntryImpl.class", + "/modules/java.base/jdk/internal/classfile/impl/EntryMap.class", + "/modules/java.base/jdk/internal/classfile/impl/Util.class", + "/modules/java.base/java/lang/classfile/constantpool/LoadableConstantEntry.class", + "/modules/java.base/java/lang/classfile/constantpool/ClassEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$AbstractRefEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$AbstractNamedEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$ClassEntryImpl.class", + "/modules/java.base/java/util/function/Consumer.class", + "/modules/java.base/java/lang/classfile/ClassFile.class", + "/modules/java.base/jdk/internal/classfile/impl/ClassFileImpl.class", + "/modules/java.base/java/lang/classfile/ClassFileBuilder.class", + "/modules/java.base/java/lang/classfile/ClassBuilder.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractDirectBuilder.class", + "/modules/java.base/jdk/internal/classfile/impl/DirectClassBuilder.class", + "/modules/java.base/jdk/internal/classfile/impl/AttributeHolder.class", + "/modules/java.base/java/lang/classfile/Superclass.class", + "/modules/java.base/jdk/internal/classfile/impl/SuperclassImpl.class", + "/modules/java.base/java/lang/classfile/attribute/SourceFileAttribute.class", + "/modules/java.base/jdk/internal/classfile/impl/UnboundAttribute$UnboundSourceFileAttribute.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractAttributeMapper$SourceFileMapper.class", + "/modules/java.base/jdk/internal/classfile/impl/BoundAttribute.class", + "/modules/java.base/java/lang/classfile/MethodBuilder.class", + "/modules/java.base/jdk/internal/classfile/impl/MethodInfo.class", + "/modules/java.base/jdk/internal/classfile/impl/TerminalMethodBuilder.class", + "/modules/java.base/jdk/internal/classfile/impl/DirectMethodBuilder.class", + "/modules/java.base/java/lang/classfile/constantpool/NameAndTypeEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$AbstractRefsEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$NameAndTypeEntryImpl.class", + "/modules/java.base/java/lang/classfile/constantpool/MemberRefEntry.class", + "/modules/java.base/java/lang/classfile/constantpool/FieldRefEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$AbstractMemberRefEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$FieldRefEntryImpl.class", + "/modules/java.base/java/lang/invoke/InvokerBytecodeGenerator$ClassData.class", + "/modules/java.base/java/lang/classfile/CodeBuilder.class", + "/modules/java.base/jdk/internal/classfile/impl/LabelContext.class", + "/modules/java.base/jdk/internal/classfile/impl/TerminalCodeBuilder.class", + "/modules/java.base/jdk/internal/classfile/impl/DirectCodeBuilder.class", + "/modules/java.base/java/lang/classfile/CodeElement.class", + "/modules/java.base/java/lang/classfile/PseudoInstruction.class", + "/modules/java.base/java/lang/classfile/instruction/CharacterRange.class", + "/modules/java.base/java/lang/classfile/instruction/LocalVariable.class", + "/modules/java.base/java/lang/classfile/instruction/LocalVariableType.class", + "/modules/java.base/jdk/internal/classfile/impl/DirectCodeBuilder$DeferredLabel.class", + "/modules/java.base/java/lang/classfile/BufWriter.class", + "/modules/java.base/jdk/internal/classfile/impl/BufWriterImpl.class", + "/modules/java.base/java/lang/classfile/Label.class", + "/modules/java.base/java/lang/classfile/instruction/LabelTarget.class", + "/modules/java.base/jdk/internal/classfile/impl/LabelImpl.class", + "/modules/java.base/sun/invoke/util/VerifyType.class", + "/modules/java.base/java/lang/classfile/Opcode.class", + "/modules/java.base/java/lang/classfile/Opcode$Kind.class", + "/modules/java.base/java/lang/classfile/constantpool/MethodRefEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$MethodRefEntryImpl.class", + "/modules/java.base/sun/invoke/empty/Empty.class", + "/modules/java.base/jdk/internal/classfile/impl/BytecodeHelpers.class", + "/modules/java.base/jdk/internal/classfile/impl/UnboundAttribute$AdHocAttribute.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractAttributeMapper$CodeMapper.class", + "/modules/java.base/java/lang/classfile/FieldBuilder.class", + "/modules/java.base/jdk/internal/classfile/impl/TerminalFieldBuilder.class", + "/modules/java.base/jdk/internal/classfile/impl/DirectFieldBuilder.class", + "/modules/java.base/java/lang/classfile/CustomAttribute.class", + "/modules/java.base/jdk/internal/classfile/impl/AnnotationReader.class", + "/modules/java.base/java/util/ListIterator.class", + "/modules/java.base/java/util/ImmutableCollections$ListItr.class", + "/modules/java.base/jdk/internal/classfile/impl/StackMapGenerator.class", + "/modules/java.base/jdk/internal/classfile/impl/StackMapGenerator$Frame.class", + "/modules/java.base/jdk/internal/classfile/impl/StackMapGenerator$Type.class", + "/modules/java.base/jdk/internal/classfile/impl/RawBytecodeHelper.class", + "/modules/java.base/jdk/internal/classfile/impl/RawBytecodeHelper$CodeRange.class", + "/modules/java.base/jdk/internal/classfile/impl/ClassHierarchyImpl.class", + "/modules/java.base/java/lang/classfile/ClassHierarchyResolver.class", + "/modules/java.base/jdk/internal/classfile/impl/ClassHierarchyImpl$ClassLoadingClassHierarchyResolver.class", + "/modules/java.base/jdk/internal/classfile/impl/ClassHierarchyImpl$CachedClassHierarchyResolver.class", + "/modules/java.base/java/lang/classfile/ClassHierarchyResolver$ClassHierarchyInfo.class", + "/modules/java.base/jdk/internal/classfile/impl/ClassHierarchyImpl$ClassHierarchyInfoImpl.class", + "/modules/java.base/java/lang/classfile/ClassReader.class", + "/modules/java.base/jdk/internal/classfile/impl/ClassReaderImpl.class", + "/modules/java.base/jdk/internal/util/ModifiedUtf.class", + "/modules/java.base/java/lang/invoke/MethodHandles$Lookup$ClassDefiner.class", + "/modules/java.base/java/lang/IncompatibleClassChangeError.class", + "/modules/java.base/java/lang/NoSuchMethodError.class", + "/modules/java.base/java/lang/invoke/BootstrapMethodInvoker.class", + "/modules/java.base/java/lang/invoke/AbstractValidatingLambdaMetafactory.class", + "/modules/java.base/java/lang/invoke/InnerClassLambdaMetafactory.class", + "/modules/java.base/java/lang/invoke/MethodHandleInfo.class", + "/modules/java.base/java/lang/invoke/InfoFromMemberName.class", + "/modules/java.base/java/util/ImmutableCollections$Access.class", + "/modules/java.base/jdk/internal/access/JavaUtilCollectionAccess.class", + "/modules/java.base/java/lang/classfile/Interfaces.class", + "/modules/java.base/jdk/internal/classfile/impl/InterfacesImpl.class", + "/modules/java.base/java/lang/invoke/TypeConvertingMethodAdapter.class", + "/modules/java.base/java/lang/invoke/DirectMethodHandle$Constructor.class", + "/modules/java.base/jdk/internal/access/JavaLangInvokeAccess.class", + "/modules/java.base/java/lang/invoke/VarHandle$AccessMode.class", + "/modules/java.base/java/lang/invoke/VarHandle$AccessType.class", + "/modules/java.base/java/lang/invoke/Invokers$Holder.class", + "/modules/java.base/jdk/internal/module/ModuleInfo.class", + "/modules/java.base/java/io/DataInput.class", + "/modules/java.base/java/io/DataInputStream.class", + "/modules/java.base/jdk/internal/module/ModuleInfo$CountingDataInput.class", + "/modules/java.base/sun/nio/ch/NativeThread.class", + "/modules/java.base/jdk/internal/misc/Blocker.class", + "/modules/java.base/sun/nio/ch/Util.class", + "/modules/java.base/sun/nio/ch/Util$BufferCache.class", + "/modules/java.base/sun/nio/ch/IOStatus.class", + "/modules/java.base/jdk/internal/util/ByteArray.class", + "/modules/java.base/java/lang/invoke/VarHandles.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsShorts$ByteArrayViewVarHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsShorts$ArrayHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleGuards.class", + "/modules/java.base/java/lang/invoke/VarForm.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsChars$ByteArrayViewVarHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsChars$ArrayHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsInts$ByteArrayViewVarHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsInts$ArrayHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsFloats$ByteArrayViewVarHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsFloats$ArrayHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsLongs$ByteArrayViewVarHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsLongs$ArrayHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsDoubles$ByteArrayViewVarHandle.class", + "/modules/java.base/java/lang/invoke/VarHandleByteArrayAsDoubles$ArrayHandle.class", + "/modules/java.base/java/lang/invoke/VarHandle$AccessDescriptor.class", + "/modules/java.base/jdk/internal/module/ModuleInfo$ConstantPool.class", + "/modules/java.base/jdk/internal/module/ModuleInfo$ConstantPool$Entry.class", + "/modules/java.base/jdk/internal/module/ModuleInfo$ConstantPool$IndexEntry.class", + "/modules/java.base/java/nio/charset/StandardCharsets.class", + "/modules/java.base/sun/nio/cs/US_ASCII.class", + "/modules/java.base/sun/nio/cs/ISO_8859_1.class", + "/modules/java.base/sun/nio/cs/UTF_16BE.class", + "/modules/java.base/sun/nio/cs/UTF_16LE.class", + "/modules/java.base/sun/nio/cs/UTF_16.class", + "/modules/java.base/sun/nio/cs/UTF_32BE.class", + "/modules/java.base/sun/nio/cs/UTF_32LE.class", + "/modules/java.base/sun/nio/cs/UTF_32.class", + "/modules/java.base/jdk/internal/module/ModuleInfo$ConstantPool$ValueEntry.class", + "/modules/java.base/java/lang/module/ModuleDescriptor$Builder.class", + "/modules/java.base/java/lang/module/ModuleDescriptor$Modifier.class", + "/modules/java.base/java/lang/reflect/AccessFlag.class", + "/modules/java.base/java/lang/reflect/AccessFlag$Location.class", + "/modules/java.base/java/lang/module/ModuleDescriptor$Requires$Modifier.class", + "/modules/java.base/java/lang/module/ModuleDescriptor$Requires.class", + "/modules/java.base/java/util/HashMap$KeySet.class", + "/modules/java.base/java/util/HashMap$KeyIterator.class", + "/modules/java.base/jdk/internal/module/Checks.class", + "/modules/java.base/java/util/ArrayList$Itr.class", + "/modules/java.base/java/lang/module/ModuleDescriptor$Provides.class", + "/modules/java.base/java/util/Collections$UnmodifiableCollection.class", + "/modules/java.base/java/util/Collections$UnmodifiableSet.class", + "/modules/java.base/java/util/HashMap$Values.class", + "/modules/java.base/java/util/HashMap$ValueIterator.class", + "/modules/java.base/java/util/ImmutableCollections$SetN$SetNIterator.class", + "/modules/java.base/jdk/internal/module/ModuleInfo$Attributes.class", + "/modules/java.base/jdk/internal/module/ModuleReferences.class", + "/modules/java.base/java/lang/module/ModuleReader.class", + "/modules/java.base/sun/nio/fs/UnixUriUtils.class", + "/modules/java.base/java/net/URI$Parser.class", + "/modules/java.base/java/lang/module/ModuleReference.class", + "/modules/java.base/jdk/internal/module/ModuleReferenceImpl.class", + "/modules/java.base/java/lang/module/ModuleDescriptor$Exports.class", + "/modules/java.base/java/lang/module/ModuleDescriptor$Opens.class", + "/modules/java.base/sun/nio/fs/UnixException.class", + "/modules/java.base/java/io/IOException.class", + "/modules/java.base/jdk/internal/loader/ArchivedClassLoaders.class", + "/modules/java.base/jdk/internal/loader/ClassLoaders$BootClassLoader.class", + "/modules/java.base/java/lang/ClassLoader$ParallelLoaders.class", + "/modules/java.base/java/util/WeakHashMap.class", + "/modules/java.base/java/util/WeakHashMap$Entry.class", + "/modules/java.base/java/util/WeakHashMap$KeySet.class", + "/modules/java.base/java/security/Principal.class", + "/modules/java.base/jdk/internal/loader/URLClassPath.class", + "/modules/java.base/java/net/URLStreamHandlerFactory.class", + "/modules/java.base/java/net/URL$DefaultFactory.class", + "/modules/java.base/jdk/internal/access/JavaNetURLAccess.class", + "/modules/java.base/sun/net/www/ParseUtil.class", + "/modules/java.base/java/net/URLStreamHandler.class", + "/modules/java.base/sun/net/www/protocol/file/Handler.class", + "/modules/java.base/sun/net/util/IPAddressUtil.class", + "/modules/java.base/sun/net/util/IPAddressUtil$MASKS.class", + "/modules/java.base/sun/net/www/protocol/jar/Handler.class", + "/modules/java.base/jdk/internal/module/ServicesCatalog.class", + "/modules/java.base/jdk/internal/loader/AbstractClassLoaderValue.class", + "/modules/java.base/jdk/internal/loader/ClassLoaderValue.class", + "/modules/java.base/jdk/internal/loader/BuiltinClassLoader$LoadedModule.class", + "/modules/java.base/jdk/internal/module/DefaultRoots.class", + "/modules/java.base/java/util/Spliterator.class", + "/modules/java.base/java/util/HashMap$HashMapSpliterator.class", + "/modules/java.base/java/util/HashMap$ValueSpliterator.class", + "/modules/java.base/java/util/stream/StreamSupport.class", + "/modules/java.base/java/util/stream/BaseStream.class", + "/modules/java.base/java/util/stream/Stream.class", + "/modules/java.base/java/util/stream/PipelineHelper.class", + "/modules/java.base/java/util/stream/AbstractPipeline.class", + "/modules/java.base/java/util/stream/ReferencePipeline.class", + "/modules/java.base/java/util/stream/ReferencePipeline$Head.class", + "/modules/java.base/java/util/stream/StreamOpFlag.class", + "/modules/java.base/java/util/stream/StreamOpFlag$Type.class", + "/modules/java.base/java/util/stream/StreamOpFlag$MaskBuilder.class", + "/modules/java.base/java/util/EnumMap.class", + "/modules/java.base/java/lang/Class$ReflectionData.class", + "/modules/java.base/java/lang/Class$Atomic.class", + "/modules/java.base/java/lang/PublicMethods$MethodList.class", + "/modules/java.base/java/lang/PublicMethods$Key.class", + "/modules/java.base/sun/reflect/annotation/AnnotationParser.class", + "/modules/java.base/jdk/internal/reflect/MethodHandleAccessorFactory.class", + "/modules/java.base/jdk/internal/reflect/MethodHandleAccessorFactory$LazyStaticHolder.class", + "/modules/java.base/java/lang/invoke/BoundMethodHandle.class", + "/modules/java.base/java/lang/invoke/ClassSpecializer.class", + "/modules/java.base/java/lang/invoke/BoundMethodHandle$Specializer.class", + "/modules/java.base/jdk/internal/vm/annotation/Stable.class", + "/modules/java.base/java/lang/invoke/ClassSpecializer$SpeciesData.class", + "/modules/java.base/java/lang/invoke/BoundMethodHandle$SpeciesData.class", + "/modules/java.base/java/lang/invoke/ClassSpecializer$Factory.class", + "/modules/java.base/java/lang/invoke/BoundMethodHandle$Specializer$Factory.class", + "/modules/java.base/java/lang/invoke/SimpleMethodHandle.class", + "/modules/java.base/java/lang/NoSuchFieldException.class", + "/modules/java.base/java/lang/invoke/BoundMethodHandle$Species_L.class", + "/modules/java.base/java/lang/invoke/DirectMethodHandle$Accessor.class", + "/modules/java.base/java/lang/invoke/DelegatingMethodHandle.class", + "/modules/java.base/java/lang/invoke/DelegatingMethodHandle$Holder.class", + "/modules/java.base/java/lang/invoke/LambdaFormEditor.class", + "/modules/java.base/java/lang/invoke/LambdaFormEditor$TransformKey.class", + "/modules/java.base/java/lang/invoke/LambdaFormBuffer.class", + "/modules/java.base/java/lang/invoke/LambdaFormEditor$Transform.class", + "/modules/java.base/jdk/internal/reflect/DirectMethodHandleAccessor.class", + "/modules/java.base/java/util/stream/Collectors.class", + "/modules/java.base/java/util/stream/Collector$Characteristics.class", + "/modules/java.base/java/util/EnumSet.class", + "/modules/java.base/java/util/RegularEnumSet.class", + "/modules/java.base/java/util/stream/Collector.class", + "/modules/java.base/java/util/stream/Collectors$CollectorImpl.class", + "/modules/java.base/java/util/function/BiConsumer.class", + "/modules/java.base/java/lang/invoke/DirectMethodHandle$Interface.class", + "/modules/java.base/java/lang/classfile/constantpool/InterfaceMethodRefEntry.class", + "/modules/java.base/jdk/internal/classfile/impl/AbstractPoolEntry$InterfaceMethodRefEntryImpl.class", + "/modules/java.base/java/util/function/BinaryOperator.class", + "/modules/java.base/java/util/stream/ReduceOps.class", + "/modules/java.base/java/util/stream/TerminalOp.class", + "/modules/java.base/java/util/stream/ReduceOps$ReduceOp.class", + "/modules/java.base/java/util/stream/StreamShape.class", + "/modules/java.base/java/util/stream/Sink.class", + "/modules/java.base/java/util/stream/TerminalSink.class", + "/modules/java.base/java/util/stream/ReduceOps$AccumulatingSink.class", + "/modules/java.base/java/util/stream/ReduceOps$Box.class", + "/modules/java.base/java/util/HashMap$KeySpliterator.class", + "/modules/java.base/java/util/function/Predicate.class", + "/modules/java.base/java/util/stream/ReferencePipeline$StatelessOp.class", + "/modules/java.base/java/util/stream/Sink$ChainedReference.class", + "/modules/java.base/jdk/internal/module/ModuleResolution.class", + "/modules/java.base/java/util/stream/FindOps.class", + "/modules/java.base/java/util/stream/FindOps$FindSink.class", + "/modules/java.base/java/util/stream/FindOps$FindSink$OfRef.class", + "/modules/java.base/java/util/stream/FindOps$FindOp.class", + "/modules/java.base/java/util/Spliterators.class", + "/modules/java.base/java/util/Spliterators$IteratorSpliterator.class", + "/modules/java.base/java/lang/module/Configuration.class", + "/modules/java.base/java/lang/module/Resolver.class", + "/modules/java.base/java/lang/ModuleLayer.class", + "/modules/java.base/java/util/SequencedSet.class", + "/modules/java.base/java/util/LinkedHashSet.class", + "/modules/java.base/java/util/SequencedMap.class", + "/modules/java.base/java/util/LinkedHashMap.class", + "/modules/java.base/java/lang/module/ResolvedModule.class", + "/modules/java.base/jdk/internal/module/ModuleLoaderMap$Mapper.class", + "/modules/java.base/jdk/internal/loader/AbstractClassLoaderValue$Memoizer.class", + "/modules/java.base/jdk/internal/module/ServicesCatalog$ServiceProvider.class", + "/modules/java.base/java/util/concurrent/CopyOnWriteArrayList.class", + "/modules/java.base/java/lang/ModuleLayer$Controller.class", + "/modules/java.base/jdk/internal/module/ModuleBootstrap$SafeModuleFinder.class", + "/modules/java.base/jdk/internal/vm/ContinuationSupport.class", + "/modules/java.base/jdk/internal/vm/Continuation$Pinned.class", + "/modules/java.base/sun/launcher/LauncherHelper.class", + "/modules/java.base/sun/net/util/URLUtil.class", + "/modules/java.base/jdk/internal/loader/URLClassPath$Loader.class", + "/modules/java.base/jdk/internal/loader/URLClassPath$FileLoader.class", + "/modules/java.base/jdk/internal/loader/Resource.class", + "/modules/java.base/java/io/FileCleanable.class", + "/modules/java.base/sun/nio/ByteBuffered.class", + "/modules/java.base/java/security/SecureClassLoader$CodeSourceKey.class", + "/modules/java.base/java/security/PermissionCollection.class", + "/modules/java.base/java/security/Permissions.class", + "/modules/java.base/java/lang/NamedPackage.class", + "/modules/java.base/jdk/internal/misc/MethodFinder.class", + "/modules/java.base/java/lang/Readable.class", + "/modules/java.base/java/nio/CharBuffer.class", + "/modules/java.base/java/nio/HeapCharBuffer.class", + "/modules/java.base/java/nio/charset/CoderResult.class", + "/modules/java.base/java/util/IdentityHashMap$IdentityHashMapIterator.class", + "/modules/java.base/java/util/IdentityHashMap$KeyIterator.class", + "/modules/java.base/java/lang/Shutdown.class", + "/modules/java.base/java/lang/Shutdown$Lock.class"); +} From 29633a1280ce1f83a525c740084e3cf70b807e55 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Tue, 1 Jul 2025 22:32:45 +0200 Subject: [PATCH 08/11] Updates based on feedback. --- .../jdk/internal/jimage/ImageReader.java | 2 +- .../internal/module/SystemModuleFinders.java | 57 +++++++++++-------- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java b/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java index 97ef47d7ca2e5..9f4c28c79c536 100644 --- a/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java +++ b/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java @@ -552,7 +552,7 @@ ImageLocation getLocation() { } /** - * Returns the JRY file system compatible name of this node (e.g. + * Returns the JRT file system compatible name of this node (e.g. * {@code "/modules/java.base/java/lang/Object.class"} or {@code * "/packages/java.lang"}). */ diff --git a/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java b/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java index 6eaf5e7a2819b..ab05a7c07b98c 100644 --- a/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java +++ b/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java @@ -44,6 +44,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.Spliterator; @@ -59,8 +60,6 @@ import jdk.internal.util.StaticProperty; import jdk.internal.module.ModuleHashes.HashSupplier; -import static java.util.Objects.requireNonNull; - /** * The factory for SystemModules objects and for creating ModuleFinder objects * that find modules in the runtime image. @@ -209,7 +208,7 @@ public static ModuleFinder ofSystem() { } /** - * Parses the module-info.class of all module in the runtime image and + * Parses the {@code module-info.class} of all modules in the runtime image and * returns a ModuleFinder to find the modules. * * @apiNote The returned ModuleFinder is thread safe. @@ -219,7 +218,7 @@ private static ModuleFinder ofModuleInfos() { Map nameToAttributes = new HashMap<>(); Map nameToHash = new HashMap<>(); - getModuleAttributes().forEach(attrs -> { + allModuleAttributes().forEach(attrs -> { nameToAttributes.put(attrs.descriptor().name(), attrs); ModuleHashes hashes = attrs.recordedHashes(); if (hashes != null) { @@ -248,24 +247,33 @@ private static ModuleFinder ofModuleInfos() { return new SystemModuleFinder(mrefs, nameToModule); } - private static Stream getModuleAttributes() { + /** + * Parses the {@code module-info.class} of all modules in the runtime image and + * returns a stream of {@link ModuleInfo.Attributes Attributes} for them. The + * returned attributes are in no specific order. + */ + private static Stream allModuleAttributes() { // System reader is a singleton and should not be closed by callers. ImageReader reader = SystemImage.reader(); - return getNode(reader, "/modules") - .getChildNames() - .map(mn -> getNode(reader, mn + "/module-info.class")) - // This fails with ISE if the node isn't a resource (corrupt JImage). - .map(reader::getResourceBuffer) - .map(bb -> ModuleInfo.read(bb, null)); + try { + return reader.findNode("/modules").getChildNames().map(mn -> loadModuleAttributes(reader, mn)); + } catch (IOException e) { + throw new Error("Error reading root /modules entry", e); + } } // The nodes we are processing must exist (every module must have a module-info.class). - private static ImageReader.Node getNode(ImageReader reader, String name) { + private static ModuleInfo.Attributes loadModuleAttributes(ImageReader reader, String moduleName) { + Exception err = null; try { - return requireNonNull(reader.findNode(name)); - } catch (IOException e) { - throw new IllegalStateException("ImageReader node must exist: " + name, e); + ImageReader.Node node = reader.findNode(moduleName + "/module-info.class"); + if (node != null && node.isResource()) { + return ModuleInfo.read(reader.getResourceBuffer(node), null); + } + } catch (IOException | UncheckedIOException e) { + err = e; } + throw new Error("Missing or invalid module-info.class for module: " + moduleName, err); } /** @@ -289,7 +297,7 @@ private static class SystemModuleFinder implements ModuleFinder { @Override public Optional find(String name) { - requireNonNull(name); + Objects.requireNonNull(name); return Optional.ofNullable(nameToModule.get(name)); } @@ -402,7 +410,7 @@ private static class SystemModuleReader implements ModuleReader { * if not found. */ private boolean containsResource(String resourcePath) throws IOException { - requireNonNull(resourcePath); + Objects.requireNonNull(resourcePath); if (closed) throw new IOException("ModuleReader is closed"); ImageReader imageReader = SystemImage.reader(); @@ -443,19 +451,18 @@ private InputStream toInputStream(ByteBuffer bb) { // ## -> ByteBuffer? } /** - * Returns the node for the given resource, or {@code null} if not found. + * Returns the node for the given resource if found. If the name references + * a non-resource node, then an {@link Optional#empty() empty optional} is + * returned even if a non-resource node exists with the given name. */ private Optional findResourceNode(ImageReader reader, String name) throws IOException { - requireNonNull(name); + Objects.requireNonNull(name); if (closed) { throw new IOException("ModuleReader is closed"); } String nodeName = "/modules/" + module + "/" + name; - Optional node = Optional.ofNullable(reader.findNode(nodeName)); - if (node.isPresent() && !node.get().isResource()) { - throw new IllegalStateException("Not a resource node: " + node.get()); - } - return node; + ImageReader.Node node = reader.findNode(nodeName); + return node != null && node.isResource() ? Optional.of(node) : Optional.empty(); } @Override @@ -466,7 +473,7 @@ public Optional read(String name) throws IOException { @Override public void release(ByteBuffer bb) { - requireNonNull(bb); + Objects.requireNonNull(bb); ImageReader.releaseByteBuffer(bb); } From 9d9e62bab37dabcf97bbb5e6c2c375fd98965744 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Wed, 2 Jul 2025 00:08:48 +0200 Subject: [PATCH 09/11] Feedback changes. * Making some simple tests parametrized. * Revert change to JImageReadTest (it passes either way, so keep it simple). --- .../jdk/internal/jimage/ImageReaderTest.java | 42 ++++++++++++------- .../jdk/internal/jimage/JImageReadTest.java | 2 +- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/test/jdk/jdk/internal/jimage/ImageReaderTest.java b/test/jdk/jdk/internal/jimage/ImageReaderTest.java index 07a5bb4f68eb7..4b4fc944421d1 100644 --- a/test/jdk/jdk/internal/jimage/ImageReaderTest.java +++ b/test/jdk/jdk/internal/jimage/ImageReaderTest.java @@ -27,6 +27,8 @@ import jdk.test.lib.util.JarBuilder; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.opentest4j.TestSkippedException; import tests.Helper; import tests.JImageGenerator; @@ -74,23 +76,33 @@ public class ImageReaderTest { "com.bar.Two")); private final Path image = buildJImage(IMAGE_ENTRIES); - @Test - public void testModuleDirectories() throws IOException { + @ParameterizedTest + @ValueSource(strings = { + "/", + "/modules", + "/modules/modfoo", + "/modules/modbar", + "/modules/modfoo/com", + "/modules/modfoo/com/foo", + "/modules/modfoo/com/foo/bar"}) + public void testModuleDirectories_expected(String name) throws IOException { try (ImageReader reader = ImageReader.open(image)) { - // Basic directory expectations. - assertDir(reader, "/"); - assertDir(reader, "/modules"); - assertDir(reader, "/modules/modfoo"); - assertDir(reader, "/modules/modbar"); - assertDir(reader, "/modules/modfoo/com"); - assertDir(reader, "/modules/modfoo/com/foo"); - assertDir(reader, "/modules/modfoo/com/foo/bar"); + assertDir(reader, name); + } + } - assertAbsent(reader, "/modules/"); - assertAbsent(reader, "/modules/unknown"); - assertAbsent(reader, "/modules/modbar/"); - assertAbsent(reader, "/modules/modbar//com"); - assertAbsent(reader, "/modules/modbar/com/"); + @ParameterizedTest + @ValueSource(strings = { + "", + "//", + "/modules/", + "/modules/unknown", + "/modules/modbar/", + "/modules/modfoo//com", + "/modules/modfoo/com/"}) + public void testModuleNodes_absent(String name) throws IOException { + try (ImageReader reader = ImageReader.open(image)) { + assertAbsent(reader, name); } } diff --git a/test/jdk/jdk/internal/jimage/JImageReadTest.java b/test/jdk/jdk/internal/jimage/JImageReadTest.java index 1f77b4cc324ad..da5d4f19bc1d9 100644 --- a/test/jdk/jdk/internal/jimage/JImageReadTest.java +++ b/test/jdk/jdk/internal/jimage/JImageReadTest.java @@ -25,7 +25,7 @@ * @test * @summary Unit test for libjimage JIMAGE_Open/Read/Close * @modules java.base/jdk.internal.jimage - * @run testng/othervm JImageReadTest + * @run testng JImageReadTest */ import java.io.File; From a8b68c7b87ee8e1369a69ce19310ce0545883fc6 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Wed, 2 Jul 2025 10:34:43 +0200 Subject: [PATCH 10/11] Small feedback related tweaks. --- .../share/classes/jdk/internal/jimage/ImageReader.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java b/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java index 9f4c28c79c536..154b0d5eaa0b9 100644 --- a/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java +++ b/src/java.base/share/classes/jdk/internal/jimage/ImageReader.java @@ -204,11 +204,11 @@ private SharedImageReader(Path imagePath, ByteOrder byteOrder) throws IOExceptio // TODO (review note): Given there are ~30,000 nodes in the image, is setting an initial capacity a good idea? this.nodes = new HashMap<>(); // TODO (review note): These should exist under all circumstances, but there's - // probably a more robust way of getting it these offsets. + // probably a more robust way of getting at these offsets. this.modulesStringOffset = findLocation("/modules/java.base").getModuleOffset(); this.packagesStringOffset = findLocation("/packages/java.lang").getModuleOffset(); - // Node creation is very lazy, se can just make the top-level directories + // Node creation is very lazy, so we can just make the top-level directories // now without the risk of triggering the building of lots of other nodes. Directory packages = newDirectory(PACKAGES_ROOT); nodes.put(packages.getName(), packages); @@ -540,8 +540,8 @@ protected Node(String name, BasicFileAttributes fileAttrs) { this.fileAttrs = Objects.requireNonNull(fileAttrs); } - // A node is completed when all its direct children have been built. As - // such, non-directory nodes are always complete. + // A node is completed when all its direct children have been built. + // As such, non-directory nodes are always complete. boolean isCompleted() { return true; } From df1705ce91cc8e3a386c67250df23c4b97c23160 Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Wed, 2 Jul 2025 15:49:00 +0200 Subject: [PATCH 11/11] Feedback related changes. --- .../jdk/internal/module/SystemModuleFinders.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java b/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java index ab05a7c07b98c..be985452314b5 100644 --- a/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java +++ b/src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java @@ -256,14 +256,14 @@ private static Stream allModuleAttributes() { // System reader is a singleton and should not be closed by callers. ImageReader reader = SystemImage.reader(); try { - return reader.findNode("/modules").getChildNames().map(mn -> loadModuleAttributes(reader, mn)); + return reader.findNode("/modules").getChildNames().map(mn -> readModuleAttributes(reader, mn)); } catch (IOException e) { throw new Error("Error reading root /modules entry", e); } } - // The nodes we are processing must exist (every module must have a module-info.class). - private static ModuleInfo.Attributes loadModuleAttributes(ImageReader reader, String moduleName) { + // Every module is required to have a valid module-info.class. + private static ModuleInfo.Attributes readModuleAttributes(ImageReader reader, String moduleName) { Exception err = null; try { ImageReader.Node node = reader.findNode(moduleName + "/module-info.class"); @@ -452,23 +452,22 @@ private InputStream toInputStream(ByteBuffer bb) { // ## -> ByteBuffer? /** * Returns the node for the given resource if found. If the name references - * a non-resource node, then an {@link Optional#empty() empty optional} is - * returned even if a non-resource node exists with the given name. + * a non-resource node, then {@code null} is returned. */ - private Optional findResourceNode(ImageReader reader, String name) throws IOException { + private ImageReader.Node findResourceNode(ImageReader reader, String name) throws IOException { Objects.requireNonNull(name); if (closed) { throw new IOException("ModuleReader is closed"); } String nodeName = "/modules/" + module + "/" + name; ImageReader.Node node = reader.findNode(nodeName); - return node != null && node.isResource() ? Optional.of(node) : Optional.empty(); + return node != null && node.isResource() ? node : null; } @Override public Optional read(String name) throws IOException { ImageReader reader = SystemImage.reader(); - return findResourceNode(reader, name).map(reader::getResourceBuffer); + return Optional.ofNullable(findResourceNode(reader, name)).map(reader::getResourceBuffer); } @Override