Welcome to Software Development on Codidact!
Will you help us build our independent community of developers helping developers? We're small and trying to grow. We welcome questions about all aspects of software development, from design to code to QA and more. Got questions? Got answers? Got code you'd like someone to review? Please join us.
What is the purpose of having a wrapper class for JarFile in Spring-boot-loader?
My question relates to Spring-boot-loader v2.6.15.
For the purposes of this question, I will refer to Spring's custom JarFile class as CustomJarFile to avoid confusion.
Context
I am reusing some of the jar package code in spring-boot-loader. I noticed that they define their own CustomJarFile implementation that (most relevant to this question) overrides the close() method like so:
@Override
public void close() throws IOException {
if (this.closed) {
return;
}
super.close();
if (this.type == JarFileType.DIRECT) {
this.rootFile.close();
}
this.closed = true;
}
They also define a JarFileWrapper class that does not override the close method.
The custom JarUrlConnection#get() method notably returns an instance with the wrapper rather than the jar itself like so:
static JarURLConnection get(URL url, JarFile jarFile) throws IOException {
// some stuff here...
return new JarURLConnection(url, jarFile.getWrapper(), jarEntryName); // JarFile here is the custom class
}
Testing the code without the wrapper class by returning a JarUrlConnection with the CustomJarFile itself rather than its wrapper results in a "zip file closed" exception.
This is reasonable given that when you call close() on the wrapper, it will call the close() method of the ZipFile class. However, if the JarFile itself is sent, it will call the close() method of the JarFile since it is overriden.
Notably, The JavaDoc of the wrapper class states:
A wrapper used to create a copy of a {@link CustomJarFile} so that it can be safely closed without closing the original.
Question
I do not quite understand the purpose of using a wrapper here. If the goal is to just avoid closing the CustomJarFile, wouldn't simply not overriding the close() method in CustomJarFile (allowing it to propogate to ZipFile) have the same effect as using a wrapper? It feels like we just added a redundant layer.
While debugging, I've noticed that the close() method of CustomJarFile is never called between startup and shutdown of the service which makes me further believe that the wrapper is pointless if you simply remove the overriden close method in CustomJarFile.
1 answer
Why does the wrapper exists and why doesn't it override close()?
Apparently, this wrapper was introduced to "Attempt to fix memory leak in JarFile class". The memory leak issue was caused by a change in java 11 that introduced FinalizableResource which made it so that any JarFile implementation that override close() will end up with finalizer based cleanup.
Ultimately, too many JarFile instances are being created, especially given they used to wrap the JarFile in a JarFile, without ever being cleaned by the garbage collector since FinalizableResource doesn't deference it.
The simplest way to tackle this was to create a wrapper class that doesn't override close() which would be created/returned instead of JarFile, essentially working around the problem.
Why was the JarFile wrapped in itself before the wrapper was added?
In the question I said the close() method in JarFile is never called which made sense since it was never returned. However, it seems JarFile is actually exposed through JarURLConnection.getJarFile() and therefore may get closed and run you into trouble. I assume the exposure is intentional since that's not what was changed. My testing missed it, but the point here is that situations definitely exist where it could be called.
Java got rid of problematic code?
Well, apparently Java didn't like the change any more than Spring did and FinalizableResource was removed in literally the first java 12 tag: jdk-12-ga. I currently use Java 17 and it still does not exist.
Is the wrapper still necessary?
Unclear... It may still be necessary for the same reason JarFile was wrapped in itself before. It may not since the language has changed a lot since. Either way, I've learned to be happy with the wrapper and coexist :)
0 comment threads
0 comment threads