-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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 flatMap inner fused poll crash not cancelling the upstream #5792
Conversation
Codecov Report
@@ Coverage Diff @@
## 2.x #5792 +/- ##
============================================
- Coverage 96.29% 96.26% -0.04%
+ Complexity 5809 5806 -3
============================================
Files 634 634
Lines 41607 41609 +2
Branches 5770 5771 +1
============================================
- Hits 40066 40055 -11
+ Misses 614 612 -2
- Partials 927 942 +15
Continue to review full report at Codecov.
|
@@ -482,6 +482,9 @@ void drainLoop() { | |||
Exceptions.throwIfFatal(ex); | |||
is.dispose(); | |||
errs.addThrowable(ex); | |||
if (!delayErrors) { | |||
upstream.cancel(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is bug fix, but what's the reason for renaming s
to upstream
?
It looks not important to revert but I guess minimizing fix PRs saves time for those who review them while going through changelog :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It reads nicer now.
This PR fixes the lack of upstream
cancel()
call when an inner, fused source'squeue.poll()
crashes in a non-delayed error mode.Unit tests were added to verify
Observable.flatMap
,Flowable.flatMapIterable
andObservable.flatMapIterable
as well.Fixes #5791