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 concatMapSingle & concatMapMaybe dispose-cleanup crash #5928

Merged
merged 1 commit into from
Mar 23, 2018

Conversation

akarnokd
Copy link
Member

This PR fixes the accidental logical mistake in the cancel/dispose logic of the new concatMapSingle and concatMapMaybe operators of both Flowable and Observable where the internal queue cleanup should happen in a serialized fashion only, which is the state when the work-in-progress counter changes from 0 to 1 ensured by a == check.

Fixes #5927

@akarnokd akarnokd added this to the 2.2 milestone Mar 23, 2018
@akarnokd
Copy link
Member Author

This is severe enough to release 2.1.12 as a hotfix version ASAP, although the mistakes are in the newly introduced operators. I've done a quick search for this pattern and doesn't happen in older code luckily. Also this is on the dispose path and doesn't really the performance in the normal operation mode (i.e., when data is allowed to flow through till the end without cancellation).

@codecov
Copy link

codecov bot commented Mar 23, 2018

Codecov Report

Merging #5928 into 2.x will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #5928      +/-   ##
============================================
+ Coverage     98.22%   98.25%   +0.02%     
- Complexity     6017     6019       +2     
============================================
  Files           656      656              
  Lines         44040    44040              
  Branches       6102     6102              
============================================
+ Hits          43260    43270      +10     
+ Misses          237      231       -6     
+ Partials        543      539       -4
Impacted Files Coverage Δ Complexity Δ
...ternal/operators/mixed/FlowableConcatMapMaybe.java 100% <100%> (ø) 2 <0> (ø) ⬇️
...rnal/operators/mixed/ObservableConcatMapMaybe.java 100% <100%> (ø) 3 <0> (ø) ⬇️
...nal/operators/mixed/ObservableConcatMapSingle.java 100% <100%> (ø) 3 <0> (ø) ⬇️
...ernal/operators/mixed/FlowableConcatMapSingle.java 100% <100%> (ø) 2 <0> (ø) ⬇️
.../io/reactivex/internal/schedulers/IoScheduler.java 89.24% <0%> (-3.23%) 9% <0%> (ø)
...rnal/operators/flowable/FlowableFlatMapSingle.java 92.93% <0%> (-2.72%) 2% <0%> (ø)
...activex/internal/observers/QueueDrainObserver.java 97.43% <0%> (-2.57%) 21% <0%> (-1%)
...ernal/operators/flowable/FlowableFromIterable.java 93.04% <0%> (-2.14%) 5% <0%> (ø)
...nternal/operators/observable/ObservableWindow.java 98% <0%> (-2%) 3% <0%> (ø)
...activex/internal/schedulers/ScheduledRunnable.java 98.07% <0%> (-1.93%) 29% <0%> (-1%)
... and 26 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 63572c7...2ffc416. 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.

[2.1.11] NPE in SpscLinkedArrayQueue.lvElement
2 participants