63

The javadoc for Stream states:

Streams have a BaseStream.close() method and implement AutoCloseable, but nearly all stream instances do not actually need to be closed after use. Generally, only streams whose source is an IO channel (such as those returned by Files.lines(Path, Charset)) will require closing. Most streams are backed by collections, arrays, or generating functions, which require no special resource management. (If a stream does require closing, it can be declared as a resource in a try-with-resources statement.)

Therefore, the vast majority of the time one can use Streams in a one-liner, like collection.stream().forEach(System.out::println); but for Files.lines and other resource-backed streams, one must use a try-with-resources statement or else leak resources.

This strikes me as error-prone and unnecessary. As Streams can only be iterated once, it seems to me that there is no a situation where the output of Files.lines should not be closed as soon as it has been iterated, and therefore the implementation should simply call close implicitly at the end of any terminal operation. Am I mistaken?

assylias
  • 297,541
  • 71
  • 621
  • 741
MikeFHay
  • 7,264
  • 4
  • 24
  • 43
  • 2
    In my experience, streams that auto-close when you don't want them to are almost impossible to work with. You can't re-open what already was closed *for* you. Mark, reset, seek. You can read some data more than once with the same stream depending on implementation. – ebyrob Dec 03 '15 at 17:25
  • 4
    @ebyrob not with that stream – assylias Dec 03 '15 at 17:33
  • 4
    Not better than a simple try-with-resource, but if you really need to do it with a single expression: http://stackoverflow.com/a/31179709/2711488 – Holger Dec 03 '15 at 19:28
  • 1
    I would point out that *all* streams in java land are not reusable, FWIW... – rogerdpack Jan 02 '18 at 17:07

4 Answers4

77

Yes, this was a deliberate decision. We considered both alternatives.

The operating design principle here is "whoever acquires the resource should release the resource". Files don't auto-close when you read to EOF; we expect files to be closed explicitly by whoever opened them. Streams that are backed by IO resources are the same.

Fortunately, the language provides a mechanism for automating this for you: try-with-resources. Because Stream implements AutoCloseable, you can do:

try (Stream<String> s = Files.lines(...)) {
    s.forEach(...);
}

The argument that "it would be really convenient to auto-close so I could write it as a one-liner" is nice, but would mostly be the tail wagging the dog. If you opened a file or other resource, you should also be prepared to close it. Effective and consistent resource management trumps "I want to write this in one line", and we chose not to distort the design just to preserve the one-line-ness.

Brian Goetz
  • 76,505
  • 17
  • 128
  • 138
  • 3
    I guess the rationale here is that if there is an unhandled exception, the Stream might not be "read all the way" and then the underlying handle will be "never closed." So this avoids that problem. Too bad it breaks up Stream chaining, and is confusing because "most other streams" don't need this paradigm. So when do you use Try-with-Resources with Objects of type Stream? Sometimes...but then again not other times. Appears the #close method is never called in a normal pipeline, even when the pipeline is "finished" ... – rogerdpack Jan 02 '18 at 19:43
  • 16
    In my opinion this is hard to notice. It is not in the Files.lines() javadoc and Eclipse does not warn about the resource not being closed if you put a terminating operation in the same line and you don't have the Stream as a variable. – aalku Feb 16 '18 at 08:22
  • 1
    Hi, I've a use case where I want to expose the Stream returned by Files.lines().map(parseIntoInternalRepresentation) to the callers because the internal representation is super heavy on memory. I think it's better not to materialize the stream into a collection and let the callers decide what additional operations they want to chain to reduce memory. Is it OK to expose this stream as long as I mention in the documentation that the callers of the API need to use it with try-with-resources? Wondering what the best practice is here. – user2103008 Mar 26 '19 at 05:08
16

I have more specific example in addition to @BrianGoetz answer. Don't forget that the Stream has escape-hatch methods like iterator(). Suppose you are doing this:

Iterator<String> iterator = Files.lines(path).iterator();

After that you may call hasNext() and next() several times, then just abandon this iterator: Iterator interface perfectly supports such use. There's no way to explicitly close the Iterator, the only object you can close here is the Stream. So this way it would work perfectly fine:

try(Stream<String> stream = Files.lines(path)) {
    Iterator<String> iterator = stream.iterator();
    // use iterator in any way you want and abandon it at any moment
} // file is correctly closed here.
Tagir Valeev
  • 87,515
  • 18
  • 194
  • 305
5

In addition if you want "one line write". You can just do this:

Files.readAllLines(source).stream().forEach(...);

You can use it if you are sure that you need entire file and the file is small. Because it isn't a lazy read.

Oliv
  • 8,930
  • 2
  • 43
  • 70
1

If you're lazy like me and don't mind the "if an exception is raised, it will leave the file handle open" you could wrap the stream in an autoclosing stream, something like this (there may be other ways):

  static Stream<String> allLinesCloseAtEnd(String filename) throws IOException {
    Stream<String> lines = Files.lines(Paths.get(filename));
    Iterator<String> linesIter = lines.iterator();

    Iterator it = new Iterator() {
      @Override
      public boolean hasNext() {
        if (!linesIter.hasNext()) {
          lines.close(); // auto-close when reach end
          return false;
        }
        return true;
      }

      @Override
      public Object next() {
        return linesIter.next();
      }
    };
    return StreamSupport.stream(Spliterators.spliteratorUnknownSize(it, Spliterator.DISTINCT), false);
  }
rogerdpack
  • 50,731
  • 31
  • 212
  • 332
  • 1
    This doesn’t work. There is no guaranty that a stream consumes all elements. There are short-circuiting operations like `find…()` or `…Match(…)`, also `limit(…)` and `takeWhile(…)`. If an application terminates the stream with `iterator()` or `spliterator()`, there’s also no guaranty that it will iterate to the end. So your solution serves only a few use cases while significantly reducing efficiency. – Holger Jan 02 '18 at 21:35
  • Also good points, thanks! (works if you read through all the lines, but if that's not the case, good to not use this). Or perhaps some will consider it a feature that you can pass the stream, for instance, out of a method that opens it, and still have it self-close gracefully if/when it's eventually used up :) – rogerdpack Jan 02 '18 at 21:50