Revisiting MayHoldCloseableResource

[Brian Goetz]

After discussion both in the EG and within Oracle, here's where I think 
this should land:

  - Eliminate MayHoldCloseableResource, which is fundamentally flawed 
unless the spec for AutoCloseable is fixed, in which case it becomes 
unnecessary.
  - Fix the spec for AutoCloseable.
  - Eliminate @HoldsResource, not because it is intrinsically bad, but 
because it is a half solution and we should work towards a complete one 
for 9, likely using something like JSR-308 checkers.  The Eclipse 
inspector can be adjusted to work with the new APIs.
  - Make BaseStream implement AutoCloseable, with additional verbiage 
regarding closing.
  - Add additional spec to Files.{walk,lines,etc}, and to combinators 
like concat, regarding their close behavior.

The thing that pushed me over the edge from the various "work around the 
problem" solutions is the existence of flatMap:

   Files.walk(dir)
        .flatMap(Files::lines)
        ...

where each entry resulting from walk() will turn into a stream creation 
and traversal within the flatMap implementation, and flatMap itself 
needs a way to ensure it is deterministically closed.  This pushes 
strongly for having a close() method on Stream, which, if you're going 
to have that, you might as well be AutoCloseable to make life easier for 
users.

The sole challenge here is to keep people from thinking that they *need* 
to close all streams, and therefore mangle their code in bad ways to 
satisfy this perceived need.


On 8/2/2013 7:39 PM, Brian Goetz wrote:
> There seems to be consensus that the current strawman for MayHoldCloseableResoruce and @HoldsResource is too flawed to proceed.
>
> Flaw #1 relates to @HoldsResource.  While there seems to be consensus that annotations are reasonable for imparting information like this which is suited to static analysis tools, the current proposal (a) confusingly leaks through into subclass Javadoc and (b) seems too weak.  Conclusion: just dropping @HR from the current scheme (leaving only MHCR <: AC) is an improvement; we can revisit what the correct annotations for hinting at resourceful-ness later.
>
> Flaw #2 relates to MHCR.  The real problem is that AutoCloseable is overspecified; it says "a resource that must be closed."  This is an overreach; what j.l.AC should say is "has a close method which is called automatically on exit from a TWR block."  Because of this overspecify, subclassing doesn't help, because if an AC must be closed, so must a MHCR.  Conclusion: dropping MHCR and just having BaseStream <: AC is an improvement.
>
> Flaw #3 relates to the spec of AC.  The conclusion of the core library maintainers is that it is acceptable to correct the spec, and replace the "must" with "may".  This leaves us with a fixed AC, and BaseStream <: AC.  This seems strictly better than where we are now.  Let's call this "Option 1."
>
> But, it is possible we might still choose to punt further for now.
>
> Observation: the need for Stream to have a close() method comes about because of the desire to have static factory methods that will create streams that encapsulate a resource that requires closing, when there is no way to get access to that underlying resource.  Such as Files.walk(dir) or Files.lines(path).
>
> For other cases, like BufferedReader.lines(), the BR itself is AC, so users can handle resource management like:
>
>    try (BufferedReader b = ...) {
>       b.lines()...
>    }
>
> and there is no need to close the *stream* at all, since the user has access to the actual closeable resource.  It is only the static methods such as Files.list(dir), which don't provide the user access in this way.  The methods we provide are:
>
> Files.list(Path)
> Files.walk(Path, maxDepth, options)
> Files.walk(Path, options)
> Files.find(Path, maxDepth, BiPredicate<Path, BasicFileAttributes>, options)
> Files.lines(Path)
>
> Colin @ Google pointed out that if the spliterators for these streams were lazy (didn't open the resource until the terminal began), and all terminal ops called the close hook in a finally block after processing, then we wouldn't need to be AC, since there would be no way to leak a closeable resource.  (This would move resource-not-found exceptions from the factory method to the terminal op.)
>
> Still, having a close() method and *not* being AC may look weird to the users.
>
> Let's call this "Option 2": have a close() method, don't extend AC, make sure that all terminals call close, make sure that all static resource-holding stream factories produce late-binding spliterators.
>
> Option 3 would be to simply get rid of these methods (note that Files.list and Files.lines are pretty simple wrappers around existing AC abstractions, DirectoryStream and BufferedReader.)
>
> Orthogonal to Options 2 and 3 would be fixing the spec of AC.
>
>