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

1.x: observeOn - fix in-sequence termination/unsubscription #3768

Merged

Conversation

akarnokd
Copy link
Member

This fixes observeOn not completing immediately if the downstream requested exactly the remaining amount. The original version required a new request from downstream to trigger the delivery of the terminal event.

This also fixes the rare failure in testNoMoreRequestsAfterUnsubscribe because the unsubscription is now checked in-sequence and doesn't trigger the unwanted replenishment request.

@@ -834,4 +834,16 @@ public void testErrorDelayedAsync() {
ts.assertError(TestException.class);
ts.assertNotCompleted();
}

@Test
public void requestExactCompletesImmediately() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it tests "immediately", this test passes without changes in this PR.

Maybe you can avoid going to another thread and awaitTerminalEvent()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, should request 0 upfront and wait till the range completes before requesting 10. I'll change this to use the TestScheduler.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the test which now clearly fails on master.

@artem-zinnatullin
Copy link
Contributor

👍 for changes, not sure about test

@akarnokd akarnokd force-pushed the ObserveOnRequestBoundaryCompletionFix branch from 672fa8e to e98b20d Compare March 15, 2016 10:17
@artem-zinnatullin
Copy link
Contributor

👍

1 similar comment
@stevegury
Copy link
Member

👍

akarnokd added a commit that referenced this pull request Mar 16, 2016
…tionFix

1.x: observeOn - fix in-sequence termination/unsubscription
@akarnokd akarnokd merged commit f7321d2 into ReactiveX:1.x Mar 16, 2016
@akarnokd akarnokd deleted the ObserveOnRequestBoundaryCompletionFix branch March 16, 2016 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants