Discussion: easier Stream closing

[Tagir Valeev]

> At that point, the real question is why not call close() at the end of all terminal operations by wrapping each implementation in a try-with-resources ?

Now, it will be incompatible change. Also, this won't work with
iterator() and spliterator() terminal operations. I believe it's best
described by Brian in this SO answer:
https://stackoverflow.com/a/34073306/4856258

Quoted here, for convenience:

> 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.

With best regards,
Tagir Valeev.

>
> Rémi
>
> ----- Original Message -----
> > From: "Brian Goetz" <brian.goetz at oracle.com>
> > To: "Tagir Valeev" <amaembo at gmail.com>, "core-libs-dev" <core-libs-dev at openjdk.java.net>
> > Sent: Lundi 27 Septembre 2021 22:41:00
> > Subject: Re: Discussion: easier Stream closing
>
> > In Java 8, I think we were reluctant to lean on the idiom of "pass me a
> > lambda and I'll pass it the confined data"), because Java developers
> > were already struggling to understand lambdas.  But now that we're
> > mostly over that hurdle, API points that accept Consumer<Something> are
> > a powerful way to gain confinement (as we've seen in StackWalker, and
> > also in the explorations of ScopeLocal in Loom.)  So I have no objection
> > to this idiom.
> >
> > I have some concern that putting these side-by-side with existing
> > Files.walk(...) methods will be a source of confusion, creating two
> > different ways to do the same thing.
> >
> > I would rather not add anything new to BaseStream; it was a mistake to
> > expose at all, and I'd rather see it deprecated than add more to it.
> > However, adding it individually to the Stream implementations is
> > equivalent, and doesn't have this problem.
> >
> > The consumeAndClose approach is clever, in that it adds one API point
> > that works for all streams, rather than having to add a new API point
> > for every factory of a closeable stream; on the other hand, it is
> > dramatically less discoverable, and so requires more education to get
> > people to use it (and, as you say, has the exception problem.)
> >
> > On 9/26/2021 5:27 AM, Tagir Valeev wrote:
> >> Hello!
> >>
> >> With current NIO file API, a very simple problem to get the list of
> >> all files in the directory requires some ceremony:
> >>
> >> List<Path> paths;
> >> try (Stream<Path> stream = Files.list(Path.of("/etc"))) {
> >>      paths = stream.toList();
> >> }
> >>
> >> If we skip try-with-resources, we may experience OS file handles leak,
> >> so it's desired to keep it. Yet, it requires doing boring stuff. In
> >> this sense, classic new File("/etc").list() was somewhat more
> >> convenient (despite its awful error handling).
> >>
> >> I like how this problem is solved in StackWalker API [1]: it limits
> >> the lifetime of the Stream by requiring a user-specified function to
> >> consume it. After the function is applied, the stream is closed
> >> automatically. We could add a similar overload to the Files API. As an
> >> additional feature, we could also translate all UncheckedIOException's
> >> back to IOException for more uniform exception processing:
> >>
> >> /**
> >>   * @param dir  The path to the directory
> >>   * @param function  function to apply to the stream of directory files
> >>   * @param <T> type of the result
> >>   * @return result of the function
> >>   * @throws IOException if an I/O error occurs when opening the directory, or
> >>   * UncheckedIOException is thrown by the supplied function.
> >>   */
> >> public static <T> T list(Path dir, Function<? super Stream<Path>, ?
> >> extends T> function) throws IOException {
> >>      try (Stream<Path> stream = Files.list(dir)) {
> >>          return function.apply(stream);
> >>      }
> >>      catch (UncheckedIOException exception) {
> >>          throw exception.getCause();
> >>      }
> >> }
> >>
> >> In this case, getting the List of all files in the directory will be
> >> as simple as
> >>
> >> List<Path> paths = Files.list(Path.of("/etc"), Stream::toList);
> >> This doesn't limit the flexibility. For example, if we need only file
> >> names instead of full paths, we can do this:
> >> List<Path> paths = Files.list(Path.of("/etc"), s ->
> >> s.map(Path::getFileName).toList());
> >>
> >> Alternatively, we could enhance the BaseStream interface in a similar
> >> way. It won't allow us to translate exceptions, but could be useful
> >> for every stream that must be closed after consumption:
> >>
> >> // in BaseStream:
> >>
> >> /**
> >>   * Apply a given function to this stream, then close the stream.
> >>   * No further operation on the stream will be possible after that.
> >>   *
> >>   * @param function function to apply
> >>   * @param <R> type of the function result
> >>   * @return result of the function
> >>   * @see #close()
> >>   */
> >> default <R> R consumeAndClose(Function<? super S, ? extends R> function) {
> >>      try(@SuppressWarnings("unchecked") S s = (S) this) {
> >>          return function.apply(s);
> >>      }
> >> }
> >>
> >> The usage would be a little bit longer but still more pleasant than
> >> explicit try-with-resources:
> >>
> >> List<Path> list = Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);
> >>
> >> On the other hand, in this case, we are free to put intermediate
> >> operations outside of consumeAndClose, so we won't need to nest
> >> everything inside the function. Only terminal operation should be
> >> placed inside the consumeAndClose. E.g., if we need file names only,
> >> like above, we can do this:
> >>
> >> List<Path> list =
> >> Files.list(Path.of("/etc")).map(Path::getFileName).consumeAndClose(Stream::toList);
> >>
> >> What do you think?
> >>
> >>
> >> [1]
> > > https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/StackWalker.html#walk(java.util.function.Function)