Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid JarURLConnection resource leak in AbstractFileResolvingResource.exists() #34528

Closed
reckart opened this issue Mar 3, 2025 · 3 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@reckart
Copy link

reckart commented Mar 3, 2025

The AbstractFileResolvingResource in Spring 5.3.39 has a resource leak (seems to still be present at least in 6.2.2).

The leak occurs when the exists() method is called on a UrlResource obtained from a JAR/ZIP file which is not on the classpath.

exists() will open a connection and may then call a variation of getContentLengthLong() on it to obtain the size of the resource to check if it exists by comparing the size to 0.

Leak: Calling getContentLengthLong() on a JarURLConnection opens the underlying JAR file. However, that file is then not closed anymore.

The problem can be checked e.g. by using https://github.com/jenkinsci/lib-file-leak-detector

Here a stack using Spring 5.3.39

#8 /var/folders/mr/5jj8y27x76s6zd22d78gqcfw0000gn/T/junit-4912917014845844348/zip/hash-in-file.zip by thread:ForkJoinPool-1-worker-1 on Mon Mar 03 11:41:06 CET 2025
at java.base/java.io.RandomAccessFile.<init>(RandomAccessFile.java:214)
at java.base/java.util.zip.ZipFile$Source.<init>(ZipFile.java:1312)
at java.base/java.util.zip.ZipFile$Source.get(ZipFile.java:1277)
at java.base/java.util.zip.ZipFile$CleanableResource.<init>(ZipFile.java:709)
at java.base/java.util.zip.ZipFile.<init>(ZipFile.java:243)
at java.base/java.util.zip.ZipFile.<init>(ZipFile.java:172)
at java.base/java.util.jar.JarFile.<init>(JarFile.java:347)
at java.base/sun.net.www.protocol.jar.URLJarFile.<init>(URLJarFile.java:103)
at java.base/sun.net.www.protocol.jar.URLJarFile.getJarFile(URLJarFile.java:72)
at java.base/sun.net.www.protocol.jar.JarFileFactory.get(JarFileFactory.java:168)
at java.base/sun.net.www.protocol.jar.JarFileFactory.getOrCreate(JarFileFactory.java:91)
at java.base/sun.net.www.protocol.jar.JarURLConnection.connect(JarURLConnection.java:132)
at java.base/sun.net.www.protocol.jar.JarURLConnection.getContentLengthLong(JarURLConnection.java:202)
at org.springframework.core.io.AbstractFileResolvingResource.exists(AbstractFileResolvingResource.java:69)

The issue e.g. prevents temporary folders from unit tests containing ZIP resources accessed through Spring to be deleted on Windows.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 3, 2025
@jhoeller
Copy link
Contributor

jhoeller commented Mar 3, 2025

What would you expect AbstractFileResolvingResource to do at that point? We could only really open an InputStream and close() it right away, like we do for our fallback existence check already, and even that step is not guaranteed to free any actual resources (but comes at the expense of opening the content stream first). URLConnection is not an API designed for specifically managing the resource lifecycle there. To some degree, it even suggests to keep the resource handles open for faster subsequent access.

It seems that the only reliable way to avoid such leaks is to manage direct file handles, avoiding any URLConnection to begin with. Are you manually performing the UrlResource access there, or is this implicitly happening within the framework somewhere?

@reckart
Copy link
Author

reckart commented Mar 3, 2025

I am retrieving the UrlResource from a PatternMatchingResourcePatternResolver() which I use to access JAR/ZIP files. Is this happening within the framework - if you consider PatternMatchingResourcePatternResolver() to be part of the framework, maybe yes.

I believe it could be considered to introduce a special handling for JarURLConnection like it exists for HttpURLConnection - and in this branch directly go to the getInputStream().close() branch.

A more involved approach could also be to hoist the access to the JAR into the AbstractFileResolvingResource itself. I.e. if the file is a JAR/ZIP residing on the file system, the archive could be opened explicitly, checked for the existence of the resource and be closed again.

@jhoeller jhoeller changed the title Resource leak in AbstractFileResolvingResource Avoid JarURLConnection resource leak in AbstractFileResolvingResource.exists() Mar 3, 2025
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 3, 2025
@jhoeller jhoeller self-assigned this Mar 3, 2025
@jhoeller jhoeller added this to the 6.2.4 milestone Mar 3, 2025
@jhoeller
Copy link
Contributor

jhoeller commented Mar 3, 2025

I've tweaked this to simply skip the getContentLengthLong() call on a JarURLConnection, always going to the getInputStream().close() branch instead. This will be available in the upcoming 6.2.4 snapshot, feel free to give it an early try!

FWIW this goes along nicely with the recent #34217 in 6.2.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants