Avoiding OperatorObserveOn from calling subscriber.onNext(..) after unsu... #1409
Conversation
|
RxJava-pull-requests #1360 SUCCESS |
| @@ -136,7 +136,22 @@ private void pollQueue() { | |||
| if (v == null) { | |||
| break; | |||
| } | |||
| on.accept(observer, v); | |||
|
|
|||
| switch (on.kind(v)) { | |||
There was a problem hiding this comment.
This kind switch adds about 20% overhead to the value delivery. Use on.isError and on.isCompleted instead
There was a problem hiding this comment.
@akarnokd is there an on.IsNext() ? I need that or i'd have to assume that not being error or completed is next?
There was a problem hiding this comment.
No, we use !isError() && !isCompleted(), but in this case, you'd only let an onError event through regardless of the scheduledUnsubscribe status. So:
if (on.isError(v) || !scheduledUnsubscribe.isUnsubscribed()) {
on.accept(observer, v);
}
|
What to change: L56: L75: delete line L79: This way, if an unsubscribe call comes from the child, it will stop the worker immediately, but upstream calls are scheduled. Beause the worker is stopped before the ObserveOnSubscriber.unsubscribe() is called, the scheduling of the worker unsubscription will not succeed. |
|
RxJava-pull-requests #1362 SUCCESS |
|
This looks okay. |
|
@akarnokd thanks for the help with this fix. |
|
Adding @benjchristensen |
|
Looks good to me. Merging and releasing soon. |
|
Actually ... is it possible for you to re-submit this without the round-about merges along the way? It would be less confusing to anyone looking at the history in the future. And if possible, avoid touching whitespace. |
|
@benjchristensen you mean squash the commits into one? |
|
Yes please. And if possible, revert the lines that touched whitespace and aren't involved in the fix. |
…nsubscribe(). The OperatorObserveOn operator uses a scheduler to cancel subscriptions as well as to deliver the objects passing through it's onNext(..) in the right context. Calling unsubscribe will schedule the actual unsubscription while not making sure that the child subscriber will no longer receive calls to onNext(..) after unsubscribe() returns. This fix makes sure that after unsubscribe() returns no more onNext(..) calls will be made on the child subscribers. Signed-off-by: David Marques <[email protected]>
|
@benjchristensen done! :) |
|
RxJava-pull-requests #1366 SUCCESS |
|
Thanks! |
Avoiding OperatorObserveOn from calling subscriber.onNext(..) after unsu...
...bscribe().
The OperatorObserveOn operator uses a scheduler to cancel subscriptions as well
as to deliver the objects passing through it's onNext(..) in the right context.
Calling unsubscribe will schedule the actual unsubscription while not making sure
that the child subscriber will no longer receive calls to onNext(..) after
unsubscribe() returns.
This fix makes sure that after unsubscribe() returns no more onNext(..) calls will be
made on the child subscribers.
Signed-off-by: David Marques [email protected]