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: Allow Single.zip and Maybe.zip result to be garbage collected #7196

Merged
merged 8 commits into from
Feb 25, 2021
Merged

3.x: Allow Single.zip and Maybe.zip result to be garbage collected #7196

merged 8 commits into from
Feb 25, 2021

Conversation

zireael-0
Copy link
Contributor

@zireael-0 zireael-0 commented Feb 24, 2021

Resolves #7195

@zireael-0 zireael-0 changed the title 3.x: Allow Sing.zip and Maybe.zip result to be garbage collected 3.x: Allow Single.zip and Maybe.zip result to be garbage collected Feb 24, 2021
Copy link
Member

@akarnokd akarnokd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please adjust the PR for both classes.

@akarnokd akarnokd added this to the 3.1 milestone Feb 24, 2021
@zireael-0 zireael-0 requested a review from akarnokd February 24, 2021 16:57
@zireael-0
Copy link
Contributor Author

@akarnokd Sorry, hit re-review accidentally 🙈

Copy link
Member

@akarnokd akarnokd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I forgot about the case when the consumer retains the whole array reference, thus clearing it out can cause late errors.

Let's make values non-final and null it out instead as you originally suggested.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Member

@akarnokd akarnokd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I committed a few changes to fix race conditions with the changes

@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #7196 (bcca53e) into 3.x (09b2b1b) will increase coverage by 0.03%.
The diff coverage is 95.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.x    #7196      +/-   ##
============================================
+ Coverage     99.49%   99.52%   +0.03%     
- Complexity     6752     6756       +4     
============================================
  Files           747      747              
  Lines         47350    47387      +37     
  Branches       6367     6384      +17     
============================================
+ Hits          47110    47163      +53     
+ Misses          112      101      -11     
+ Partials        128      123       -5     
Impacted Files Coverage Δ Complexity Δ
...ava3/internal/operators/single/SingleZipArray.java 98.68% <85.71%> (-1.32%) 6.00 <0.00> (ø)
...xjava3/internal/operators/maybe/MaybeZipArray.java 98.79% <87.50%> (-1.21%) 6.00 <0.00> (ø)
...main/java/io/reactivex/rxjava3/core/Scheduler.java 100.00% <100.00%> (ø) 16.00 <2.00> (ø)
...3/internal/operators/parallel/ParallelCollect.java 100.00% <100.00%> (ø) 7.00 <3.00> (ø)
...internal/operators/parallel/ParallelConcatMap.java 100.00% <100.00%> (ø) 5.00 <3.00> (ø)
...ternal/operators/parallel/ParallelDoOnNextTry.java 100.00% <100.00%> (ø) 6.00 <4.00> (ø)
...a3/internal/operators/parallel/ParallelFilter.java 100.00% <100.00%> (ø) 6.00 <4.00> (ø)
...internal/operators/parallel/ParallelFilterTry.java 100.00% <100.00%> (ø) 6.00 <4.00> (ø)
...3/internal/operators/parallel/ParallelFlatMap.java 100.00% <100.00%> (ø) 5.00 <3.00> (ø)
...al/operators/parallel/ParallelFlatMapIterable.java 100.00% <100.00%> (ø) 5.00 <3.00> (ø)
... and 29 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 09b2b1b...40922cd. Read the comment docs.

@akarnokd akarnokd merged commit 55941d9 into ReactiveX:3.x Feb 25, 2021
akarnokd added a commit that referenced this pull request Feb 25, 2021
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.

GC friendly Single.zip operator
3 participants