-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Ensuring Runnables posted with delay to a Handler are removed when unsub... #1321
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
Changes from all commits
641d4bc
0045930
09c3b62
b5655d2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
/** | ||
* Copyright 2014 Netflix, Inc. | ||
* | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
|
@@ -20,8 +20,8 @@ | |
import rx.Scheduler; | ||
import rx.Subscription; | ||
import rx.functions.Action0; | ||
import rx.functions.Action1; | ||
import rx.subscriptions.BooleanSubscription; | ||
import rx.internal.schedulers.ScheduledAction; | ||
import rx.subscriptions.CompositeSubscription; | ||
import rx.subscriptions.Subscriptions; | ||
import android.os.Handler; | ||
|
||
|
@@ -34,7 +34,7 @@ public class HandlerThreadScheduler extends Scheduler { | |
|
||
/** | ||
* Constructs a {@link HandlerThreadScheduler} using the given {@link Handler} | ||
* | ||
* | ||
* @param handler | ||
* {@link Handler} to use when scheduling actions | ||
*/ | ||
|
@@ -46,47 +46,42 @@ public HandlerThreadScheduler(Handler handler) { | |
public Worker createWorker() { | ||
return new InnerHandlerThreadScheduler(handler); | ||
} | ||
|
||
private static class InnerHandlerThreadScheduler extends Worker { | ||
|
||
private final Handler handler; | ||
private BooleanSubscription innerSubscription = new BooleanSubscription(); | ||
|
||
private final CompositeSubscription compositeSubscription = new CompositeSubscription(); | ||
|
||
public InnerHandlerThreadScheduler(Handler handler) { | ||
this.handler = handler; | ||
} | ||
|
||
@Override | ||
public void unsubscribe() { | ||
innerSubscription.unsubscribe(); | ||
compositeSubscription.unsubscribe(); | ||
} | ||
|
||
@Override | ||
public boolean isUnsubscribed() { | ||
return innerSubscription.isUnsubscribed(); | ||
return compositeSubscription.isUnsubscribed(); | ||
} | ||
|
||
@Override | ||
public Subscription schedule(final Action0 action, long delayTime, TimeUnit unit) { | ||
final Runnable runnable = new Runnable() { | ||
@Override | ||
public void run() { | ||
if (isUnsubscribed()) { | ||
return; | ||
} | ||
action.call(); | ||
} | ||
}; | ||
handler.postDelayed(runnable, unit.toMillis(delayTime)); | ||
return Subscriptions.create(new Action0() { | ||
|
||
final ScheduledAction scheduledAction = new ScheduledAction(action); | ||
scheduledAction.add(Subscriptions.create(new Action0() { | ||
@Override | ||
public void call() { | ||
handler.removeCallbacks(runnable); | ||
|
||
handler.removeCallbacks(scheduledAction); | ||
} | ||
|
||
}); | ||
})); | ||
scheduledAction.addParent(compositeSubscription); | ||
compositeSubscription.add(scheduledAction); | ||
|
||
handler.postDelayed(scheduledAction, unit.toMillis(delayTime)); | ||
|
||
return scheduledAction; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just trying to understand the problem better: why are we not returning the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because we want to cancel the individual task, not the entire worker. |
||
} | ||
|
||
@Override | ||
|
@@ -95,5 +90,4 @@ public Subscription schedule(final Action0 action) { | |
} | ||
|
||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to add the scheduledAction to the composite: