Fix JarFile resource leak in JarDetails#18385
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a file-handle leak in runtime-telemetry’s jar metadata extraction by ensuring JarFile instances are always closed and by turning JarDetails into an immutable data holder.
Changes:
- Close
JarFilevia try-with-resources inJarDetails.forUrl()and eagerly compute pom/manifest/sha1 before returning. - Remove the
EmbeddedJarDetailssubclass and replace it with static helper overloads for top-level vs embedded jars. - Refactor digest computation helpers to operate on
URL/(JarFile, JarEntry)streams instead of holding an openJarFilefield.
| urlString.substring( | ||
| "jar:file:".length(), index + 1 + entry.getValue().length()))) { |
There was a problem hiding this comment.
This won't work for paths with spaces since in url space is encoded with %20. For ideas see https://github.com/spring-projects/spring-framework/blob/3184eb3acc8e2e6e95623c43e979ef6c256887ed/spring-core/src/main/java/org/springframework/util/ResourceUtils.java#L324
This can be annoyingly complicated since the urls can be malformed. For example as far as I remember websphere by default doesn't encode spaces to %20.
There was a problem hiding this comment.
added support for spaces (and a test)
| } | ||
| } | ||
| return new JarDetails(url, new JarFile(url.getFile())); | ||
| try (JarFile jarFile = new JarFile(url.getFile())) { |
|
|
||
| static JarDetails forUrl(URL url) throws IOException { | ||
| if (url.getProtocol().equals("jar")) { | ||
| String urlString = url.toExternalForm(); |
There was a problem hiding this comment.
Tests don't seem to hit this block. Is this for handling nested jars like when you have one jar inside another jar?
JarDetails.forUrl was passing the still-URL-encoded path to new JarFile, so paths containing characters like spaces (encoded as %20) would fail to open. Decode via URI.getSchemeSpecificPart, matching the existing pattern in JarAnalyzer. Extract shared helper UrlPaths.
Adds tests that exercise the jar:file URL branch (nested archive URLs like a jar inside a war), including the path-with-spaces case.
Extracted from #18382.
Tip
For reviewers, the diff looks better with "Hide whitespace"
JarDetailsheld an openJarFilein a field and never closed it, so each loaded library leaked one file handle for the JVM's lifetime.Fix: read pom / manifest / sha1 eagerly in
forUrlinside a try-with-resources block, then pass the results into the constructor.JarDetailsbecomes an immutable data holder and theJarFileis closed before the factory returns.The
EmbeddedJarDetailssubclass existed only to override I/O methods and is no longer needed; replaced with overloaded static helpers (getPom,getManifest,computeDigest) — one variant for top-level jars, one for(JarFile, JarEntry)embedded jars.No behavior change beyond the leak fix.