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

Schedule when bug fix #4826

Merged
merged 2 commits into from
Nov 9, 2016
Merged

Schedule when bug fix #4826

merged 2 commits into from
Nov 9, 2016

Conversation

abersnaze
Copy link
Contributor

In using this in production we found a bug where some actions were dropped. I've tracked it down to the premature onCompleted and unsubscription of the completable that represents the scheduled action causes the future to get canceled before it is started.

The fix was to delay the onCompleted until the action was truly done.

I did the PR as two commits because the first one is changing all the tabs to spaces! Look at the second commit for the actual functional difference.

…k to get unsubscribed too early. Changed the code to delay the calling of onCompleted of the CompletableSubscriber until the actualAction is truly done.
static final Subscription UNSUBSCRIBED = Subscriptions.unsubscribed();

@SuppressWarnings("serial")
private static abstract class ScheduledAction extends AtomicReference<Subscription>implements Subscription {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private creates extra accessors in an Android unfriendly way. Could you change these into package-private?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want the changes to package-private in a new PR? For both 1.x and 2.x?

@codecov-io
Copy link

Current coverage is 84.19% (diff: 87.83%)

Merging #4826 into 1.x will increase coverage by 0.15%

@@                1.x      #4826   diff @@
==========================================
  Files           287        287          
  Lines         17828      17835     +7   
  Methods           0          0          
  Messages          0          0          
  Branches       2702       2702          
==========================================
+ Hits          14983      15016    +33   
+ Misses         1968       1953    -15   
+ Partials        877        866    -11   

Powered by Codecov. Last update d6bc923...6186a70

@akarnokd akarnokd added the Bug label Nov 9, 2016
@akarnokd akarnokd added this to the 1.3 milestone Nov 9, 2016
@akarnokd akarnokd merged commit 1f6c68c into ReactiveX:1.x Nov 9, 2016
@abersnaze abersnaze deleted the schedule-when-1x branch November 10, 2016 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants