Hi Tagir,
AutoCloseable.close states:
* However, implementers of this interface are strongly encouraged
* to make their {@code close} methods idempotent.
It’s possible we just did not consider this aspect too much in BaseStream.close
and we just lent on the "strongly encouraged”, which seems fine to me to
continue to do so.
We did not bother to throw ISEs from parallel/sequence/onClose primarily
because they return “this” and they are kind of harmless, but the undefined
behaviour you point out could be shut down without much concern for backwards
compatibility. We can modify them to check whether they have been linked or
consumed (while not updating the previous stage).
Wanna log an issue?
Thanks,
Paul.
> On 15 Jan 2016, at 05:35, Tagir F. Valeev <amae...@gmail.com> wrote:
>
> Hello!
>
> Current documentation for BaseStream.onClose() does not specify
> explicitly how the stream would behave if additional close handlers
> are registered after the stream is consumed or even closed. The
> implementation actually allows registering the additional close
> handlers after consumption or closure and close() method is
> idempotent: newly registered handlers are perfectly called:
>
> Stream<String> s = Stream.of("content");
> s = s.onClose(() -> System.out.println("A"));
> s.forEach(System.out::println);
> s = s.onClose(() -> System.out.println("B"));
> s.close(); // prints A and B
> s = s.onClose(() -> System.out.println("C"));
> s.close(); // prints C
>
> (removing "s =" produces the same result).
>
> I think if such behavior is intended, it should be explicitly noted in
> the specification, as third-party implementations of BaseStream
> interface should provide consistent behavior. Or at least it should be
> noted that some BaseStream implementations may have idempotent close()
> method, so the derived AutoCloseable objects (which own the BaseStream
> object) should be aware about this behavior and provide idempotent
> close() method as well. Without knowing this one may write:
>
> class MyObject implements AutoCloseable {
> private BaseStream<?, ?> s;
>
> public MyObject(BaseStream<?, ?> s) {
> this.s = s;
> }
>
> @Override
> public void close() throws Exception {
> if(s != null) {
> s.close();
> s = null;
> }
> }
> }
>
> Such code closes the stream only once, so newly registered handlers
> will not be called if MyObject::close is called the second time.
>
> However, to my opinion the current behavior is weird and should be
> changed in order not to allow registering new close handles (throwing
> IllegalStateException) when the stream is already closed, or even
> better when the stream is linked/consumed. As far as I know currently
> in JDK close handlers are registered only for non-consumed stream, so
> such change would not break existing JDK code (though may break some
> strange third party code). It should be noted that AutoCloseable
> interface discourages idempotent close() methods (though not forbids
> them).
>
> What do you think?
>
> With best regards,
> Tagir Valeev.
>
