Bug ID: JDK-8338445 jdk.internal.loader.URLClassPath may leak JarFile instance when dealing with unexpected Class-Path entry in manifest

Type: Bug
Component: core-libs
Sub-Component: java.net
Affected Version: 8

Priority: P4
Status: Resolved
Resolution: Fixed
OS: windows_10
CPU: x86_64

Submitted: 2024-08-14
Updated: 2024-09-02
Resolved: 2024-08-28

JDK 24
24 b13Fixed

ADDITIONAL SYSTEM INFORMATION :
Detected in Win 10, JDK 21.0.2+13, but according to sourcecode error is still in master and happen on all OS. However only on Windows it makes a problem in our usecase as the ZipFile can not be deleted from disk as long as JDK still has the file open.

A DESCRIPTION OF THE PROBLEM :
The constructor of JarFile registers a Cleaner that is only executed by chance if a GC runs. If no GC runs the Cleaner is not executed.
jdk.internal.loader.URLClassPath$JarLoader.getJarFile() 
https://github.com/openjdk/jdk/blob/38bd8a36704a962f0ad1052fd2ec150a61663256/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L797
calls checkJar() with an JarFile which is never closed when when System.getSecurityManager()==null
https://github.com/openjdk/jdk/blob/38bd8a36704a962f0ad1052fd2ec150a61663256/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L780

The error was reported java.nio.file.FileSystemException:  "The process cannot access the file because it is being used by another process" when the still opened zipfile is tried to be deleted.


STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
zipFile=new ZipFile(file)
use the same file as URLClasspath
zipFile.close(); // should release the filehandle
try to delete the file -> error

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
jar is closed when no longer used.
Probably just use try-with-resource block in jdk.internal.loader.URLClassPath$JarLoader.getJarFile(java.net.URL)  for the new constructed Closable JarFile
ACTUAL -
System filehandle still open after jdk.internal.loader.URLClassPath$JarLoader.checkJar(java.util.jar.JarFile) 


CUSTOMER SUBMITTED WORKAROUND :
manually invoke a Garbage Collection with System.gc();

FREQUENCY : always

Changeset: 2e174c63 Branch: master Author: Jaikiran Pai <jpai@openjdk.org> Date: 2024-08-28 09:29:18 +0000 URL: https://git.openjdk.org/jdk/commit/2e174c6367c7755d8541f9669f7f10ed89878f58
28-08-2024
A pull request was submitted for review. Branch: master URL: https://git.openjdk.org/jdk/pull/20691 Date: 2024-08-23 10:45:49 +0000
23-08-2024
> As mentioned in the original bug report (but not copied into the jira) the error was reported in https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2812 Thank you for that detail. That was crucial in understanding what is going on. What's happening here is that the application's test code is calling a File.delete() against a file which represents a jar file. On the failing instances (on Windows), the delete is failing due to something holding onto the underlying jar file. The investigation in that linked thread indicates that the JDK internal class URLClassPath (through the URLClassLoader) is playing some role. It's common and expected of the URLClassPath (through URLClassLoader) to be constructing and keeping open such JarFile instances. Going through that issue and a linked issue https://github.com/eclipse-jdt/eclipse.jdt.core/pull/2820, the fact that the reporter got past the File.delete() failure by introducing a System.gc() call workaround was a hint that there's some leak going on in some code. A JarFile instance (through ZipFile class) is backed by a RandomAccessFile. Multiple JarFile instances (of the same jar file) can be backed by the same instance of RandomAccessFile because of the ref count based caching involved in ZipFile implementation. That's all the internal details of the JDK. Another internal detail of the JDK is the URLClassPath class (which when typically used through URLClassLoader) holds strong references to a construct called JarLoader. The JarLoader is backed by an instance of JarFile. So a strong reference to a JarLoader implies a strong reference to the JarFile. When a URLClassLoader is closed, it closes the URLClassPath which then closes each of the JarLoader(s) and thus the JarFile(s). The fact that a call to GC is allowing for a "File.delete()" against a jar file to succeed is a sign that the JarFile instance wasn't strongly referenced and thus the Cleaner implementation, in ZipFile, would have closed the underlying RandomAccessFile when the JarFile got garbage collected. Another detail that helped was a comment here which states that it's always consistently the same test method and the same jar which starts showing this issue https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2766#issuecomment-2265697695. I managed to generate the same "lib8.jar" locally by running that project. Looking through it, the interesting bit is that it has a MANIFEST.MF entry with a Class-Path which looks like: > Class-Path: C:\foobar\tmp\comptest\run.1724344907640\lib/lib3.jar lib1.jar Looking at the URLClassPath code in the JDK, we have specific checks for verifying the validity of such Class-Path entries. Paying a closer attention to that code reveals that we have a potential leak there https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/loader/URLClassPath.java#L443-L453. Specifically, the code is of the form: // Otherwise, create a new Loader for the URL. Loader loader; try { loader = getLoader(url); // If the loader defines a local class path then add the // URLs as the next URLs to be opened. URL[] urls = loader.getClassPath(); if (urls != null) { push(urls); } } catch (IOException e) { // Silently ignore for now... continue; } catch (SecurityException se) { ... continue; } // Finally, add the Loader to the search path. loaders.add(loader); lmap.put(urlNoFragString, loader); When an IOException (or SecurityException) occurs while parsing and verifying the "Class-Path" entries, the URLClassPath implementation "forgets" the constructed "loader" instance representing the JarLoader and moves on. In successful cases, it adds the "loader" to the relevant collections and thus maintains the strong reference to the JarLoader. But in this failing case, that constructed "loader" leaks and is never closed. We should be closing that "loader" in the catch blocks. I added some debug logging here to verify that this is what is going on and indeed, on Windows, it shows that parsing the "Class-Path" entry raises an IOException which we then silently ignore and leak the loader: java.net.MalformedURLException: unknown protocol: c at java.base/java.net.URL.<init>(URL.java:779) at java.base/java.net.URL.<init>(URL.java:654) at java.base/jdk.internal.loader.URLClassPath$JarLoader.tryResolveFile(URLClassPath.java:978) at java.base/jdk.internal.loader.URLClassPath$JarLoader.tryResolve(URLClassPath.java:963) at java.base/jdk.internal.loader.URLClassPath$JarLoader.parseClassPath(URLClassPath.java:941) at java.base/jdk.internal.loader.URLClassPath$JarLoader.getClassPath(URLClassPath.java:920) at java.base/jdk.internal.loader.URLClassPath.getLoader(URLClassPath.java:447) at java.base/jdk.internal.loader.URLClassPath.findResource(URLClassPath.java:292) Given that I now know what the issue is, I was able to reproduce this with a trivial example which constructs a URLClassLoader with such specific jar file: import java.io.; import java.net.; public class Foo { public static void main(final String[] args) throws Exception { // launched using a specific jar file with an error raising Class-Path entry in the manifest final File jarFile = new File(args[0]); try (final URLClassLoader cl = new URLClassLoader(new URL[] {jarFile.toURI().toURL()})) { System.out.println("resource " + cl.getResource("doesnt-matter-non-existent.txt") + " from classloader " + cl); } if (!jarFile.delete()) { throw new RuntimeException("could not delete " + jarFile); } } } Adding a "loader.close()" in the catch block of that URLClassPath code gets this test to pass. I'll address this issue and take a more deeper look about the implications as well as that "Class-Path" parsing code in the coming days.
22-08-2024
[~rkale], can we ask the reporter the actual use case where they ran into the issue which prompted them to start investigating the JDK code? There's more than one place within the JDK which deals with different types of caches when it comes to jar files. The assertion that the JDK doesn't close the JarFile instance which is created through the URLClassPath when the SecurityManager isn't set can distract from the actual use case/issue that the reporter is running into. That part of the code in the URLClassPath which in the presence of a SecurityManager closes the JarFile instance, does that close() only if there's an error with the jar content. So comparing that with the case where SecurityManager isn't set isn't accurate. There are certain known issues with the jar file caching and the inability to delete such cached jars. JDK bugs already exist for those known issue. To make sure the reporter is running into the same known issues, it would be good to understand their actual use case.
20-08-2024
There shouldn't be any expectation that JAR files on the class path are closed, instead I assume the scenario here is something create its own URLClassLoader (or subclass). That API defines the close method which may help in some cases. There are long standing interactions with the URL caching that may be an issue here too.
15-08-2024