Hello! Here's a webrev: http://cr.openjdk.java.net/~tvaleev/webrev/8147505/r1/
I just added a check in AbstractPipeline.onClose and the corresponding unit-test. To me it seems that no documentation corrections are necessary, because "onClose" is already documented that it's an intermediate operation: http://download.java.net/jdk9/docs/api/java/util/stream/BaseStream.html#onClose-java.lang.Runnable- Also it's already documented: > A stream should be operated on (invoking an intermediate or terminal > stream operation) only once. This rules out, for example, "forked" > streams, where the same source feeds two or more pipelines, or > multiple traversals of the same stream. A stream implementation may > throw IllegalStateException if it detects that the stream is being > reused. http://download.java.net/jdk9/docs/api/java/util/stream/Stream.html So this already implies that invoking intermediate operation "onClose" after stream terminal operation is not allowed and may throw IllegalStateException. Please review! With best regards, Tagir Valeev. TFV> Seems that my original letter sounds confusing due to my bad English. TFV> Every occurrence of "idempotent" should be read as "not idempotent". TFV> 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? TFV> Sure: TFV> https://bugs.openjdk.java.net/browse/JDK-8147505 TFV> Thanks, TFV> Tagir Valeev. PS>> Thanks, PS>> 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. >>>
