Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subscription not disposed after onComplete with RxJava 2.x #5283

Closed
alexvictoor opened this issue Apr 13, 2017 · 5 comments
Closed

Subscription not disposed after onComplete with RxJava 2.x #5283

alexvictoor opened this issue Apr 13, 2017 · 5 comments

Comments

@alexvictoor
Copy link

Hello
I am not sure this is an issue, I would rather say this is a question.
I have noticed a different behavior between RxJava 1.x and RxJava 2.x
When an observable is completed, with RxJava 1.x subscriptions get unsubscribed:

BehaviorSubject<Object> subject = BehaviorSubject.create();
Subscription subscription = subject.subscribe();
subject.onCompleted();
System.out.println(subscription.isUnsubscribed()); // display true

However with RxJava2, the subscription is not disposed when the observable is completed:

Disposable subscription = null;
@Test
public void test() {
    BehaviorSubject subject = BehaviorSubject.create();
    Observer<? super String> observer = new Observer<String>() {
        
        public void onSubscribe(Disposable d) {
            subscription = d;
        }

        ...
    };
    
    subject.subscribe(observer);
    subject.onComplete();
    System.out.println(subscription.isDisposed());  // display false
}

Is it a wanted behavior? If so is this behavior documented somewhere?
Thanks a lot for your help

@akarnokd
Copy link
Member

Generally, if you don't dispose a disposable received via onSubscribe it may or may not report itself as disposed. The lambda version of the subscribe() does report itself disposed when it calls the lambda for a terminal event.

In practice, there shouldn't be any reason to call isDisposed outside an operator or create method because it assumes synchronous termination, or worse, it is the reactive version of Future.get().

In the reactive world, you react to termination via the appropriate onXXX method, compose in the continuation or simply side-effect via doFinally.

Sidenote: One of the smaller but ever growing regret of mine is that I let isDisposed into the API, because I knew that will eventually trigger a bunch of inconsistency bug reports and correcting them adds more overhead. I'm glad the Reactive-Streams specification opted for no isCancelled method.

@alexvictoor
Copy link
Author

Thanks a lot for your "reactivity" and your explanations!
I agree that isCancelled is less misleading.
I have the feeling that I am not the only being confused: http://stackoverflow.com/questions/41826478/do-i-have-to-unsubscribe-from-completed-observable

@Mauin
Copy link
Contributor

Mauin commented Apr 19, 2017

Related question: Observable.doOnDispose() Javadoc mentions the following:
"Note that terminal events trigger the action unless the {@code ObservableSource} is subscribed to via {@code unsafeSubscribe()}."

Is this still true or should .doFinally()/.doOnTerminate() be the preferred option?

@akarnokd
Copy link
Member

That's no longer true. The preferred operator is doFinally as it covers onError, onComplete and dispose as well.

Would you like to post a PR that removes that note? If so, please check the other base types for the equivalent operator as there might be copy-pasted all over the place.

@Mauin
Copy link
Contributor

Mauin commented Apr 19, 2017

Okay, thanks for clarifying. I can update the JavaDoc note later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants