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

2.x: add subscribeOn overload to avoid same-pool deadlock with create #5386

Merged
merged 1 commit into from
Jun 4, 2017

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Jun 1, 2017

This PR adds an overload to subscribeOn that exposes the existing feature to optionally request on the same scheduler where the subscription happened. This is necessary to avoid same-pool deadlock when the upstream contains create logic that blocks the emission thread, preventing any scheduled request to get through and leading to excess buffering or dropping data excessively. By not scheduling the request, it can directly update the emitter's request tracking and let the emitter continue.

Formerly, the existing subscribeOn automatically disabled scheduling the requests if the immediate upstream was a FlowableCreate. However, if there were operators between create and subscribeOn (as often happening on Android with composing the schedulers at the end of the chain), the subscribeOn operator run in scheduled request mode by default.

This change allows specifying this behavior through the new overload and thus distance between the create and subscribeOn operators no longer matters.

(Note that 1.x already has this overload.)

@akarnokd akarnokd added this to the 2.2 milestone Jun 1, 2017
@codecov
Copy link

codecov bot commented Jun 1, 2017

Codecov Report

Merging #5386 into 2.x will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##               2.x    #5386      +/-   ##
===========================================
+ Coverage     96.1%   96.13%   +0.03%     
- Complexity    5781     5786       +5     
===========================================
  Files          630      630              
  Lines        41195    41197       +2     
  Branches      5726     5728       +2     
===========================================
+ Hits         39591    39606      +15     
+ Misses         632      626       -6     
+ Partials       972      965       -7
Impacted Files Coverage Δ Complexity Δ
...ternal/operators/flowable/FlowableSubscribeOn.java 91.52% <100%> (ø) 2 <0> (ø) ⬇️
src/main/java/io/reactivex/Flowable.java 100% <100%> (ø) 523 <2> (+2) ⬆️
...al/operators/observable/ObservableSampleTimed.java 88.33% <0%> (-10%) 3% <0%> (ø)
...rnal/subscriptions/ArrayCompositeSubscription.java 93.33% <0%> (-6.67%) 17% <0%> (-1%)
...rnal/subscriptions/DeferredScalarSubscription.java 93.84% <0%> (-4.62%) 27% <0%> (-1%)
...rnal/operators/completable/CompletableTimeout.java 94% <0%> (-4%) 2% <0%> (ø)
...ernal/operators/maybe/MaybeTakeUntilPublisher.java 96% <0%> (-4%) 2% <0%> (ø)
...reactivex/internal/operators/single/SingleAmb.java 96.36% <0%> (-3.64%) 9% <0%> (-1%)
...vex/internal/operators/single/SingleTakeUntil.java 91.8% <0%> (-3.28%) 2% <0%> (ø)
.../internal/operators/flowable/FlowableInterval.java 94.44% <0%> (-2.78%) 3% <0%> (ø)
... and 43 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fa773c...44c0d7e. Read the comment docs.

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