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.

Sign up for GitHub

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

Jump to bottom

shareIn and cache operators #1716

Closed
wants to merge 3 commits into from
Closed

shareIn and cache operators #1716

wants to merge 3 commits into from

Conversation

RBusarow
Copy link
Contributor

Fixes #1261

elizarov added a commit that referenced this pull request Dec 25, 2019
@elizarov
ReceiveChannel.broadcast shall start lazy coroutine when closed
cdaa135 
* Documentation on broadcast operators is added that explains that the resulting BroadcastChannel shall be cancelled if it is not needed anymore.
* More tests added for various broadcast cancel/close cases.
* The only functional change is that closing a broadcast channel for lazy coroutine shall start the corresponding coroutine to give it a chance to promptly fail.
* Mark broadcast operators as obsolete. To be replaced with sharing operators on flows (see #1716).

Fixes #1713
@elizarov elizarov mentioned this pull request Dec 25, 2019
Merged
elizarov added a commit that referenced this pull request Dec 26, 2019
@elizarov
ReceiveChannel.broadcast shall start lazy coroutine when closed
5782e62 
* Documentation on broadcast operators is added that explains that the resulting BroadcastChannel shall be cancelled if it is not needed anymore.
* More tests added for various broadcast cancel/close cases.
* The only functional change is that closing a broadcast channel for lazy coroutine shall start the corresponding coroutine to give it a chance to promptly fail.
* Mark broadcast operators as obsolete. To be replaced with sharing operators on flows (see #1716).

Fixes #1713
LouisCAD
LouisCAD suggested changes Jan 8, 2020
View reviewed changes
kotlinx-coroutines-core/common/src/flow/internal/SharedFlow.kt
} else this

private fun reset() {
cache = CircularArray(cacheHistory)
Copy link
Contributor

Choose a reason for hiding this comment

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

Existing channel is not closed, which can leak the shared flow each time refCount reaches 0.
This function should close lazyChannelRef.value before assigning a new value to the lazyChannelRef variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

Believe it or not I removed that just before making the PR, thinking "well no one's collecting it, so what could leak?". Except of course the leak can go two ways.

elizarov added a commit that referenced this pull request Feb 17, 2020
@elizarov
ReceiveChannel.broadcast shall start lazy coroutine when closed
f361535 
* Documentation on broadcast operators is added that explains that the resulting BroadcastChannel shall be cancelled if it is not needed anymore.
* More tests added for various broadcast cancel/close cases.
* The only functional change is that closing a broadcast channel for lazy coroutine shall start the corresponding coroutine to give it a chance to promptly fail.
* Mark broadcast operators as obsolete. To be replaced with sharing operators on flows (see #1716).

Fixes #1713
@elizarov elizarov mentioned this pull request Mar 3, 2020
Open
@qwwdfsad qwwdfsad force-pushed the develop branch 3 times, most recently from 4a49830 to aff8202 Compare March 10, 2020 17:27
elizarov added a commit that referenced this pull request Mar 16, 2020
@elizarov
ReceiveChannel.broadcast shall start lazy coroutine when closed
b4ccb30 
* Documentation on broadcast operators is added that explains that the resulting BroadcastChannel shall be cancelled if it is not needed anymore.
* More tests added for various broadcast cancel/close cases.
* The only functional change is that closing a broadcast channel for lazy coroutine shall start the corresponding coroutine to give it a chance to promptly fail.
* Mark broadcast operators as obsolete. To be replaced with sharing operators on flows (see #1716).

Fixes #1713
@elizarov elizarov mentioned this pull request Apr 2, 2020
Closed
@LouisCAD LouisCAD mentioned this pull request May 1, 2020
Closed
@elizarov
Copy link
Contributor

In light of #2069 I'm closing this one. Thanks!

@elizarov elizarov closed this Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LouisCAD LouisCAD LouisCAD requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@RBusarow @elizarov @LouisCAD