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: fix PublishProcessor cancel/emission overflow bug #5669

Merged
merged 1 commit into from
Oct 15, 2017

Conversation

akarnokd
Copy link
Member

This PR should fix the bug that caused the test failure in #5545.

The bug manifested itself when a cancellation was happening the same time a request 1 was being fulfilled. Since the same request accounting was used for cancellation indicator, if the cancel happened between the onNext()'s get() check and decrementAndGet, this decrementAndGet decremented Long.MIN_VALUE unconditionally, which lead to a state that would appear the Subscriber still can receive events. A concurrent offer, which saves the current array of registered Subscribers, then would emit an item and overflow the Subscriber.

The fix is to use the cancellation-aware BackpressureHelper.producedCancel() utility.

Unit test were added to verify the correct behavior on both PublishProcessor and BehaviorProcessor (the latter uses different cancellation mechanism via a dedicated field).

@akarnokd akarnokd added this to the 2.2 milestone Oct 15, 2017
@codecov
Copy link

codecov bot commented Oct 15, 2017

Codecov Report

Merging #5669 into 2.x will decrease coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #5669      +/-   ##
============================================
- Coverage     96.27%   96.21%   -0.06%     
- Complexity     5815     5818       +3     
============================================
  Files           633      633              
  Lines         41553    41552       -1     
  Branches       5752     5751       -1     
============================================
- Hits          40004    39981      -23     
- Misses          612      625      +13     
- Partials        937      946       +9
Impacted Files Coverage Δ Complexity Δ
...java/io/reactivex/processors/PublishProcessor.java 98.27% <100%> (-0.02%) 45 <0> (ø)
...l/operators/observable/ObservableFlatMapMaybe.java 84.96% <0%> (-8.5%) 2% <0%> (ø)
...ernal/operators/flowable/FlowableFlatMapMaybe.java 88.88% <0%> (-5.8%) 2% <0%> (ø)
...nternal/operators/observable/ObservableCreate.java 94.01% <0%> (-4.28%) 2% <0%> (ø)
...rnal/operators/flowable/FlowableTakeLastTimed.java 96.29% <0%> (-2.78%) 2% <0%> (ø)
...nternal/operators/parallel/ParallelSortedJoin.java 92.75% <0%> (-2.18%) 2% <0%> (ø)
...ernal/operators/flowable/FlowableFromIterable.java 95.18% <0%> (-1.07%) 5% <0%> (ø)
.../operators/maybe/MaybeFlatMapIterableFlowable.java 97.54% <0%> (-0.82%) 2% <0%> (ø)
...ernal/operators/flowable/FlowableTimeoutTimed.java 98.37% <0%> (-0.82%) 3% <0%> (ø)
.../operators/observable/ObservableFlatMapSingle.java 88.8% <0%> (-0.75%) 2% <0%> (ø)
... and 19 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 cd6bc08...678c62f. 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