Stream is AutoCloseable, Stream.onClose

[Remi Forax]

On 06/20/2013 08:18 PM, Brian Goetz wrote:
> FWIW, the closest analogue to "onClose" in the current API is 
> "parallel", which modifies execution characteristics of the current 
> stream (whether it does this mutatively or functionally is a detail).
>
> Perhaps a good analogy would be: what would you call a "register a 
> close hook" method on a builder object?  Because Stream is kind of 
> like a builder.  I am not sure the onClose(closeAction) is all that 
> confusing in the context of a builder; its not a method declaration, 
> and almost no user will ever implement Stream directly, so I'm not 
> sure the confusion is all that bad?

I agree with Joe, onClose is often use for a callback (node.js and Dart 
are full of them),
I think I prefer finallyClose(AutoCloseable).

cheers,
Rémi

>
> On 6/20/2013 2:10 PM, Joe Bowbeer wrote:
>> I like the benefits, though I think the onClose name is confusing.
>>
>> In many frameworks, the onError/onExit/onEvent naming style is reserved
>> for method names in callback interfaces.  It is the prevalent convention
>> currently.  So I'd expect onClose() to be the method that is called-back
>> upon closing.
>>
>> The method name that is needed for this wrapping purpose should be
>> something more imperative, possibly mentioning its function as a "close"
>> guard.
>>
>> Some ideas below:
>>
>> .autoclose(ac)
>> .autoclosing(ac)
>>
>> .closeableStream(ac)
>>
>> --Joe
>>
>>
>> On Thu, Jun 20, 2013 at 8:22 AM, Brian Goetz <brian.goetz at oracle.com
>> <mailto:brian.goetz at oracle.com>> wrote:
>>
>>     Paul, this looks great.  I love that we can get rid of
>>     CloseableStream and DelegatingStream (two of the last remaining
>>     top-level API warts), and efficiently incorporate resource release
>>     in a predictable manner.
>>
>>
>>
>>
>>
>>     On 6/20/2013 11:17 AM, Paul Sandoz wrote:
>>
>>         Hi,
>>
>>         Having another go at this...
>>
>>         The current solution of resource management for Stream is poor
>>         (as previously mentioned):
>>
>>         1) CloseableStream/__DelegatingStream add a lot of API 
>> surface area.
>>
>>         2) Operations on CloseableStream do not return a 
>> CloseableStream.
>>
>>         3) try-with-resources always needs to be used with the
>>         CloseableStream since the Stream itself has no closeable 
>> semantics.
>>
>>         4) Stream.concat fails to propagate the closing
>>
>>
>>         If Stream extends from AutoCloseable we can address the first 3
>>         issues. In fact it can be addressed with just a close method but
>>         it is awkward to transform that into an AutoCloseable for use
>>         with try-with-resources: try (AutoCloseable ac = () ->
>>         s.close()) { … }
>>
>>         A negative point is it is no longer clear whether a stream
>>         should be closed or not, which is anyway the case for issues
>>         2/3/4. However, i don't think that should stop us trying to
>>         improve the general situation, it's not gonna be perfect but i
>>         think we can do better than what we currently have [*].
>>
>>
>>         Issue 4 can be addressed by adding a
>>         Stream.onClose(AutoCloseable ac) method.
>>
>>         Stream s = ...
>>         s = s.onClose(a).filter(...).__onClose(b).
>>         s.close(); // b is called, then a is called
>>         s.toArray(); // throws ISE
>>
>>         Stream s = ...
>>         s = s.onClose(a).onClose(b).
>>         s.close(); // b is called, then a is called
>>         s.toArray(); // throws ISE
>>
>>         The Stream.concat implementation becomes:
>>
>>                   Stream<T> cs = (a.isParallel() || b.isParallel())
>>                          ? StreamSupport.parallelStream(__split)
>>                          : StreamSupport.stream(split);
>>
>>                   return cs.onClose(() -> { a.close(); b.close(); } ) //
>>         ignoring exception handling to be brief
>>
>>
>>         The Stream.close/onClose methods enable us to specify more
>>         precisely the behaviour of Stream.close, the order in which
>>         calls to close on AutoClosable instances passed to onClose
>>         occur, and what happens if AutoCloseable.close throws an 
>> exception.
>>
>>
>>         Of course it is possible to do silly things like this:
>>
>>             s = s.conClose(a).filter(...).__onClose(s).
>>
>>         but we could detect if s is stage in the pipeline and throw 
>> an IAE.
>>
>>
>>         FWIW Stream.close/onClose is easy to implement efficiently.
>>
>>         Paul.
>>
>>         [*] The JDK world is murky, see ByteArrayInputStream.close:
>>
>>               /**
>>                * Closing a <tt>ByteArrayInputStream</tt> has no effect.
>>         The methods in
>>                * this class can be called after the stream has been
>>         closed without
>>                * generating an <tt>IOException</tt>.
>>                * <p>
>>                */
>>               public void close() throws IOException {
>>               }
>>
>>