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

3.x: Fix Flowable.switchMap not canceling properly during onNext-cancel races #6917

Merged
merged 1 commit into from
Feb 27, 2020

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Feb 27, 2020

There was an unnecessary active.lazySet(null) which prevented cancellation of the inner source under some circumstances.

More specifically, when one thread issued a cancel, the cancelled flag was set, then another thread in drain would loop around, see the cancelled flag and clear the reference. Back in the cancelling thread, the disposeInner would only see null and do nothing.

Observable.switchMap did not have this mistake. Both received unit tests to verify the correct behavior.

2.x will be fixed in a separate PR.

Resolves #6914

@akarnokd akarnokd added this to the 3.1 milestone Feb 27, 2020
@codecov
Copy link

codecov bot commented Feb 27, 2020

Codecov Report

Merging #6917 into 3.x will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.x    #6917      +/-   ##
============================================
- Coverage     99.65%   99.63%   -0.03%     
+ Complexity     6670     6666       -4     
============================================
  Files           742      742              
  Lines         47217    47216       -1     
  Branches       6367     6367              
============================================
- Hits          47055    47043      -12     
- Misses           48       57       +9     
- Partials        114      116       +2     
Impacted Files Coverage Δ Complexity Δ
...tivex/rxjava3/internal/jdk8/ParallelCollector.java 93.57% <0.00%> (-4.59%) 2.00% <0.00%> (ø%)
...xjava3/internal/observers/FutureMultiObserver.java 96.61% <0.00%> (-3.39%) 27.00% <0.00%> (-1.00%)
...ctivex/rxjava3/internal/util/QueueDrainHelper.java 97.22% <0.00%> (-2.78%) 56.00% <0.00%> (-2.00%)
...rnal/operators/flowable/FlowableFlatMapSingle.java 95.34% <0.00%> (-2.33%) 2.00% <0.00%> (ø%)
...ernal/operators/flowable/FlowableFromIterable.java 96.87% <0.00%> (-2.09%) 5.00% <0.00%> (ø%)
.../internal/disposables/ListCompositeDisposable.java 98.00% <0.00%> (-2.00%) 34.00% <0.00%> (-1.00%)
...ternal/operators/completable/CompletableMerge.java 97.29% <0.00%> (-1.36%) 2.00% <0.00%> (ø%)
...java3/internal/operators/flowable/FlowableZip.java 98.97% <0.00%> (-1.03%) 6.00% <0.00%> (ø%)
...a3/internal/operators/flowable/FlowableReplay.java 99.13% <0.00%> (-0.87%) 22.00% <0.00%> (ø%)
...3/internal/operators/flowable/FlowableGroupBy.java 99.05% <0.00%> (-0.32%) 3.00% <0.00%> (ø%)
... and 9 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 042dee3...311ec74. Read the comment docs.

@akarnokd akarnokd merged commit b34e5f6 into ReactiveX:3.x Feb 27, 2020
@akarnokd akarnokd deleted the SwitchMapCancelRaceFix branch February 27, 2020 19:48
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.

Flowable.Switchmap sometimes doesn't unsubscribe from inner stream
2 participants