Resource Leak Issue with Files.list in ai.djl.util.Utils

[yhzdys]

Description

In the ai.djl.util.Utils class of the ai.djl:api version 0.26.0, there appears to be an issue with the usage of Files.list in the getNestedModelDir method. Specifically, the stream returned by Files.list(modelDir) is not being closed properly, which may lead to file descriptor leaks.

Expected Behavior

The expected behavior is that any resources opened via Files.list(modelDir) should be closed after their use to prevent file descriptor exhaustion and ensure proper resource management.

Error Message

N/A (This is a potential resource leak issue rather than a runtime error that generates a message.)

How to Reproduce?

The issue can be observed in the code snippet provided but might not manifest as a direct error during execution. It is a best practice concern rather than a functional bug that crashes the application.

Steps to reproduce

Since this is a resource management issue, it doesn't produce a direct error trace but can be verified through code inspection or static analysis tools that check for resource leaks.

1. Inspect the code of ai.djl.util.Utils.getNestedModelDir, ai.djl.util.Utils.getCurrentEpoch, ai.djl.util.ClassLoaderUtils.scanDirectory, and ai.djl.util.ClassLoaderUtils.findImplementation.
2. Notice the lack of try-with-resources or explicit .close() calls on the Stream returned by Files.list().

What have you tried to solve it?

I propose incorporating try-with-resources to ensure the Stream is closed automatically:

public static Path getNestedModelDir(Path modelDir) {
    if (Files.isDirectory(modelDir)) {
        try (Stream<Path> paths = Files.list(modelDir)) {
            // handle actual model directory is subdirectory case
            List<Path> files = paths
                    .filter(p -> !p.getFileName().toString().startsWith("."))
                    .collect(Collectors.toList());
            if (files.size() == 1 && Files.isDirectory(files.get(0))) {
                return files.get(0);
            }
        } catch (IOException e) {
            throw new AssertionError("Failed to list files: " + modelDir, e);
        }
    }
    return modelDir.toAbsolutePath();
}

Similar modifications should be applied to the mentioned methods in Utils and ClassLoaderUtils.

Environment Info

N/A

[frankfliu]

@yhzdys

From API point of view, we should close the Stream, however, there is no resource leak in Files.list() case. The DirectoryStream object doesn't hold any resource.

[yhzdys]

😃Thank you for your response.

Files.list() relies on file descriptors to manage the underlying file system resources. However, while Java's Garbage Collection automatically handles the closure of DirectoryStream objects created by Files.list(), there's a potential risk of file descriptor exhaustion before GC is triggered.

This occurs because GC operates non-deterministically, meaning it's uncertain when exactly it will run to reclaim resources. Consequently, if file descriptors aren't properly managed and closed, there's a possibility of exhausting system resources, leading to performance issues or even system crashes.


[yhzdys]

😃Thank you for your response.  Files.list() relies on file descriptors to manage the underlying file system resources. However, while Java's Garbage Collection automatically handles the closure of DirectoryStream objects created by Files.list(), there's a potential risk of file descriptor exhaustion before GC is triggered.  This occurs because GC operates non-deterministically, meaning it's uncertain when exactly it will run to reclaim resources. Consequently, if file descriptors aren't properly managed and closed, there's a possibility of exhausting system resources, leading to performance issues or even system crashes.

Upon reviewing the discussion at Stack Overflow, I've realized my previous understanding was mistaken. The feedback there confirms that Java's GC, while eventually handling DirectoryStream objects, does not reliably or promptly close file descriptors. To avoid leaks and ensure efficient resource management, employing try-with-resources is the recommended approach.

In light of this, updating methods like ai.djl.util.Utils.getNestedModelDir() with try-with-resources would align with best practices, ensuring file descriptors are closed in a timely manner. My previous assertion lacked accuracy, and I concur with the necessity for these improvements.