Multi discrepancies #2413
Multi discrepancies #2413
Conversation
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
| value = item; // assert: even if the race occurs, we will deliver one of the items with which complete() | ||
| // has been invoked - we support only the case with a single invocation of complete() | ||
| int state = getAndUpdate(n -> n | VALUE_ARRIVED); | ||
| if ((state & REQUEST_ARRIVED) == REQUEST_ARRIVED) { |
There was a problem hiding this comment.
I am not sure why you added the masking of the value. getAndUpdate returns the value that existed before - so comparing to just REQUEST_ARRIVED in the POC was intentional.
a. in the case that we support masking makes no difference.
b. if the method is abused, and does get called more than once, the POC would still deliver only one value, and masking will deliver more than one value, and break the spec by delivering onNext after onComplete.
There was a problem hiding this comment.
I agree that the masking in line 89 is redundant.
There was a problem hiding this comment.
Aha I see, but it makes sense in the other complete() right?
public final void complete() {
int state = get();
if (((state & VALUE_ARRIVED) != VALUE_ARRIVED) && compareAndSet(state, DONE)){
downstream.onComplete();
}
}There was a problem hiding this comment.
Yes, in the other complete it reflects the logic of the POC correctly: basically, in that complete it is "if value has not arrived yet" (was worded with different conditions with OR).
But in this complete the logic must be "if request has been seen, but the value still not seen" - only in that case we are allowed to deliver the signals to downstream.
| @@ -98,6 +98,7 @@ public void nextSource() { | |||
| @Override | |||
| public void request(long n) { | |||
| if (n <= 0) { | |||
| cancel(); | |||
There was a problem hiding this comment.
This is not sufficient for the problem that has been discovered. We'll have to address it better.
There was a problem hiding this comment.
@olotenko To be clear, your point is that the sequence onError onNext is still possible as cancellation is only a best-effort task.
There was a problem hiding this comment.
Yes, you can view this as the best effort so far. But like I mentioned offline, there is another issue that needs addressing, so we may just as well postpone touching this class.
There was a problem hiding this comment.
I see, sounds like for a future PR.
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
Signed-off-by: Daniel Kec daniel.kec@oracle.com