2.x: Fix publish not requesting upon client change #6364
Conversation
| } | ||
| } | ||
|
|
||
| // if we did emit at least one element, request more to replenish the queue |
There was a problem hiding this comment.
Does not this comment need an update?
There was a problem hiding this comment.
No, this still applies if no change to the subscriber array happened.
Codecov Report
@@ Coverage Diff @@
## 2.x #6364 +/- ##
============================================
+ Coverage 98.26% 98.26% +<.01%
- Complexity 6286 6288 +2
============================================
Files 673 673
Lines 45021 45023 +2
Branches 6224 6226 +2
============================================
+ Hits 44242 44244 +2
Misses 246 246
Partials 533 533
Continue to review full report at Codecov.
|
|
|
||
| // if we did emit at least one element, request more to replenish the queue | ||
| if (d > 0) { | ||
| if (d != 0) { |
There was a problem hiding this comment.
Sorry I don't understand this particular change, how can we end up with negative d?
Otherwise it's same as previously used condition 🤔
There was a problem hiding this comment.
d should be never negative, thus != is just fine.
There was a problem hiding this comment.
but then d > 0 is okay too, or is this a performance optimization?
| TestHelper.assertBadRequestReported(Flowable.range(1, 5).publish()); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
nit: add link to original bug? #6363
There was a problem hiding this comment.
I don't usually include the issue number, that's what GitHub is for.
Due to a bug in the
Flowable.publishoperator, it is not requesting more if there was a change in the subscriber array during an emission run in the non-fused operation mode.Fixes: #6363