Potential file leak in FileSystem.read() method
#1364
Comments
pablobaxter
changed the title
FileSystem.read() methodFileSystem.read() method
|
Any chance you could reproduce this with a FakeFileSystem and its ability to check for unexpected open files? Looking at the stack trace here I don’t see the leak, though perhaps it’s in JarURLConnection or similar. |
|
@swankjesse, I believe you are right. Using the public InputStream getResourceAsStream(String name) {
Objects.requireNonNull(name);
URL url = getResource(name);
try {
return url != null ? url.openStream() : null;
} catch (IOException e) {
return null;
}
}Here, the focus is on the protected java.net.URLConnection openConnection(URL u)
throws IOException {
return new JarURLConnection(u, this);
}class JarURLInputStream extends java.io.FilterInputStream {
JarURLInputStream (InputStream src) {
super (src);
}
public void close () throws IOException {
try {
super.close();
} finally {
if (!getUseCaches()) {
jarFile.close();
}
}
}
}What's interesting here is that the I believe there is a workaround for this (by calling |
|
Great investigation! |
|
Hello @pablobaxter and @swankjesse I've also encountered this issue and I'd like to help with the file leak in I'm interested in working on this issue for an assignment. If it's unassigned, could it be assigned to me? Thanks! |
|
I'm not actively working on a solution for this yet. My idea was to approach this similarly to how Gradle approached what I believe was a similar issue with the Java API. If you intend to take this on, feel free to ping me on a PR for it. |
|
I tried a few attempts to resolve the file descriptor leak in FileSystem.read() method.
Preliminary tests indicate a successful resolution of the file descriptor leak issue for the third method, but I am still working on testing to validate the stability of this solution. |
|
I tried to devise a custom resource stream wrapper to gain better control over the useCaches flag of JarURLConnection. This is implemented to ensure that the InputStream is closed explicitly. object CustomResourceLoader {
fun getResourceAsStream(classLoader: ClassLoader, resourceName: String): InputStream? {
val resourceURL = classLoader.getResource(resourceName) ?: return null
val connection = resourceURL.openConnection()
// Explicitly disable caching
connection.useCaches = false
return connection.getInputStream().let { CustomResourceInputStream(it) }
}
}
private class CustomResourceInputStream(inputStream: InputStream) : FilterInputStream(inputStream) {
override fun close() {
try {
super.close()
} finally {
// Ensure the underlying jar file is closed
(this.`in` as? JarURLConnection)?.jarFile?.close()
}
}
}
// Usage:
val stream = CustomResourceLoader.getResourceAsStream(
ClassLoader.getSystemClassLoader(), "path/to/resource"
)
stream?.use { /* process the stream */ }
// Usage:
val stream = CustomResourceLoader.getResourceAsStream(
ClassLoader.getSystemClassLoader(), "path/to/resource"
)
stream?.use { /* process the stream */ }` |
|
@Lisi-Chen I'm surprised that just setting |
|
I decided to take a stab at this. Unit test took some effort, but it is able to confirm whether the file is cached by the |
* Fix for #1364 * Fix issue for Java8 testing * Ensure fileLeakInResourceFileSystemTest runs only if `/proc` is available
Continued investigation in file descriptor leaks led to this stack trace:
From what I've been able to gather, this is the culprit code:
From what I can tell, the
use {}lambda is not closing thesourceproperty of theRealBufferedSourceobject, but it's unclear why.The text was updated successfully, but these errors were encountered: