Java

Java

Made by DeepSource

Closeable values should not be injected via @Provides annotated methods JAVA-E1103

Bug risk
Major
sans top 25 cwe-400

Avoid marking methods that return Closeable with any dependency injection annotations such as @Provides or @Inject, as this could cause resource management problems.

This issue respects suppression via @SuppressWarnings("CloseableProvides").

Bad Practice

Consider this method that "provides" a FileOutputStream value through DI:

@Provides
FileOutputStream provideFileStream() {
    return new FileOutputStream(someFile);
}

Note that this method would return a new output stream every time it is called.

Now, what if we have some component in our application that requires a file output stream to be injected into it?

class DependencyComponent {
  private final FileOutputStream fos;

  @Inject
  DependencyComponent(FileOutputStream fos) { // fos would be provided through provideFileStream()!
     this.fos = fos; 
  }

  // ...
}

If multiple instances of DependencyComponent were to exist, provideFileStream() would be called that many times as well, meaning there would be multiple different file streams pointing to the same file. It is possible that the application could run out of file descriptors if there are too many streams opened.

If the file stream is supposed to be a singleton (if provideFileStream() were marked with @Singleton), this would mean the same stream is shared across multiple instances of DependencyComponent. This would be a different kind of problem; which instance of DependencyComponent would be responsible for closing the singleton?

For a more in-depth exploration of the problem, see this page from ErrorProne.

Recommended

Instead of injecting a resource directly, provide a wrapper that creates and destroys the resource only when needed.

class FileInterface {
    File file;

    public FileInterface(File file) {
        this.file = file;
    }

    // A file stream is created and destroyed entirely within this method alone.
    public void doWithFile(Consumer<FileOutputStream> action) {
        try (FileOutputStream stream = new FileOutputStream(this.file)) {
            action.accept(stream);
        }
    }
}

// ...

@Provides
FileInterface provideFileInterface() {
    return new FileInterface(someFile);
}

The FileInterface type now has a method doWithFile which automatically opens a file output stream, performs some action on it, and closes the stream once done. This way, the resource is only created when required.

References

  • CWE-400 - Uncontrolled resource consumption