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

No way to prevent Future.cancel(true) on unsubscribe. #3601

Closed
mopsalarm opened this issue Jan 5, 2016 · 7 comments
Closed

No way to prevent Future.cancel(true) on unsubscribe. #3601

mopsalarm opened this issue Jan 5, 2016 · 7 comments
Labels

Comments

@mopsalarm
Copy link

If i see that correctly, it looks like there is no way to prevent RxJava from calling Future.cancel(true) in ScheduledAction.FutureCompleter when unsubscribing from some subscription that runs on some thread scheduler like Schedulers.io().

Sadly, this causes problems with libraries which do not like to be interrupted (OkHttp in my case: square/okhttp#1903)

Is there some whay around this that I just dont see right now, is this by design or is this a bug?

@iNoles
Copy link

iNoles commented Jan 5, 2016

I am doing like

final Call call = client.newCall(request);
                subscriber.add(Subscriptions.create(new Action0() {
                    @Override
                    public void call() {
                        call.cancel();
                    }
                }));

@akarnokd
Copy link
Member

akarnokd commented Jan 5, 2016

Yes we do interrupted cancelling because otherwise it wouldn't be possible to interrupt a sleeping or io-blocked operation scheduled. The only exception is when the unsubscription comes from the same thread the task is scheduled in which case there is no interruption.

The problem is that this may be parameterized on multiple levels: entire system, entire scheduler, just a worker or just per task.

How are you scheduling this code and is your subscriber really unsubscribed or just runs to completion?

@mopsalarm
Copy link
Author

@akarnokd yes, I saw a few of your pull-requests on this topic which would have made this configurable, but it looked like none of those were finally merged.

I am using retrofit in conjunction with retrofit-rxjava and rxandroid. Basically i am doing something like

Observable.create(...)
  .subscribeOn(Schedulers.io())
  .unsubscribeOn(Schedulers.io())
  .observeOn(mainThread())
  .compose(bindFragment(lifecycle))
  .subscribe(...)

bindFragment is defined here [1] and simply unsubscribes from the source observable when some event occures on another (lifecycle) observable.

I actually tracked the creation of the FutureCompleter to this unsubscribe-event (debugging RxJava stacktraces are a blast!).

[1] https://github.com/trello/RxLifecycle/blob/master/rxlifecycle/src/main/java/com/trello/rxlifecycle/RxLifecycle.java#L194

@JakeWharton
Copy link
Member

Retrofit 2 does not use futures but OkHttp's native Call API (and cancel()).

@mopsalarm
Copy link
Author

I think RxJava is wrapping the call into a future which is later canceled.

@JakeWharton
Copy link
Member

Are you using Retrofit 1 or Retrofit 2? Retrofit 1 uses some ugly ductape to support RxJava which involves a FutureTask. Retrofit 2 natively and correctly supports RxJava.

@mopsalarm
Copy link
Author

I am using Retrofit 2.

But, thinking about it, I didn't track the error down to some retrofit call, but to a picasso.Downloader.load() call, which I was using with rxjava. Maybe using OkHttp directly and using this subscriber.add ... call.cancel() code will help there.
I'll try tomorrow after work and report back. Thanks for your input, this was very helpful.

Good night!

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

No branches or pull requests

4 participants