Skip to content

LazyStandaloneCoroutine contains an val reference to block #767

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

Merged
merged 1 commit into from
Nov 12, 2018

Conversation

fvasco
Copy link
Contributor

@fvasco fvasco commented Oct 29, 2018

The class LazyStandaloneCoroutine set a private val reference to the block lambda.
This reference must be used only one time, but it cannot be reclaimed from GC until the Job instance is fully unreferenced.
Jobs, Deferreds and producers should release all potentially large references as soon at possible.

@elizarov elizarov self-requested a review November 1, 2018 18:40
Copy link
Contributor

@elizarov elizarov left a comment

Choose a reason for hiding this comment

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

The code looks good for me, but for consistency purposes it should be added to all LazyXxxCoroutine classes.

@elizarov elizarov requested a review from qwwdfsad November 1, 2018 18:43
@fvasco fvasco force-pushed the LazyStandaloneCoroutine branch from 9304d60 to c775257 Compare November 6, 2018 08:09
@fvasco
Copy link
Contributor Author

fvasco commented Nov 6, 2018

This commit changes LazyDeferredCoroutine, LazyStandaloneCoroutine and LazyBroadcastCoroutine

@qwwdfsad qwwdfsad force-pushed the develop branch 3 times, most recently from bc68d63 to 3179683 Compare November 12, 2018 11:49
@qwwdfsad
Copy link
Collaborator

ChannelCoroutine is not changed, but I think it is okay as they have a different usage pattern.
I'll merge it as soon as it pass all tests

@qwwdfsad
Copy link
Collaborator

Thanks!

@qwwdfsad qwwdfsad merged commit c26e071 into Kotlin:develop Nov 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants