OutputStreamPublisher now handles close on its own. #1732
Conversation
Signed-off-by: Tomas Langer <tomas.langer@oracle.com>
Signed-off-by: Tomas Langer <tomas.langer@oracle.com>
| @@ -163,6 +195,7 @@ private void complete(Throwable t) { | |||
| subscriber.close(sub -> { | |||
| synchronized (invocationLock) { | |||
| sub.onError(t); | |||
There was a problem hiding this comment.
Lock doesn't help to prevent rule §1.7 violation, we can remove synchronization logic from publisher and wrap subscriber in io.helidon.common.reactive.SequentialSubscriber
There was a problem hiding this comment.
Not a blocker for this PR, just noticing
There was a problem hiding this comment.
1.7 violation can be seen to not be reachable in this instance.
There was a problem hiding this comment.
I'm thinking about not very probable use-case of request(-1) vs. exception in publish method
There was a problem hiding this comment.
You can see subscriber.close ensures completion signals will be signalled once and only once, as long as they always go through complete which calls subscriber.close.
There was a problem hiding this comment.
Missed that, thanks for clarification
There was a problem hiding this comment.
Pro tip: when designing routines with CPS, ensure a continuation is invoked always. (Not necessarily the same continuation for all outcomes, not necessarily immediately, but you've got to design routines with a guarantee that the continuation is invoked always) You want to not call any continuation only in very rare circumstances.
The problem with the current design of SingleSubscriberHolder is that it can omit calling the continuation, and the caller is not told that that is the case. It is the same behaviour as non-termination. The outcome is that signalCloseComplete is not guaranteed to be reachable, and the completionResult.get can wait indefinitely. The (rather long) timeout then only eventually reclaims the thread.
Signed-off-by: Tomas Langer tomas.langer@oracle.com
This was discovered by Jersey team - when using
OutputStreamPublisher, there was an infinite blocking problem with methodclose.