Clarification of BaseStream.onClose() behavior

[Tagir F. Valeev]

Hello!

Seems that my original letter sounds confusing due to my bad English.
Every occurrence of "idempotent" should be read as "not idempotent".
Sorry for this!

PS> We did not bother to throw ISEs from parallel/sequence/onClose
PS> primarily because they return “this” and they are kind of
PS> harmless, but the undefined behaviour you point out could be shut
PS> down without much concern for backwards compatibility. We can
PS> modify them to check whether they have been linked or consumed
PS> (while not updating the previous stage).

PS> Wanna log an issue?

Sure:
https://bugs.openjdk.java.net/browse/JDK-8147505

Thanks,
Tagir Valeev.

PS> Thanks,
PS> Paul.


>> On 15 Jan 2016, at 05:35, Tagir F. Valeev <amaembo at 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.
>>