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

Memory leak in LifecycleCoroutineScopeStore #136

Closed
damianw opened this issue Jun 27, 2020 · 0 comments · Fixed by #141
Closed

Memory leak in LifecycleCoroutineScopeStore #136

damianw opened this issue Jun 27, 2020 · 0 comments · Fixed by #141
Milestone
1.0.0-beta04

Comments

@damianw
Copy link

damianw commented Jun 27, 2020

Hi, loved finding your library! I was just contemplating writing something like it until I came across it and it seemed to match perfectly with what I was looking for. I did however come across a few issues that I wanted to flag to see if they're on your radar - notably with LifecycleCoroutineScopeStore.

Each access of LifecycleOwner.lifecycleScope for a distinct Lifecycle (any individual instance of a Fragment, Activity, etc) results in an insertion to map which is never removed, leaking both the Lifecycle key and the LifecycleCoroutineScope value.

A separate issue is that LifecycleCoroutineScopeStore is not thread-safe, because the "get or put" logic as implemented is not atomic. Multiple concurrent invocations of get may return separate instances of LifecycleCoroutineScope, but only one of them will get stored in map. A possible fix would be to use Kotlin's ConcurrentMap.getOrPut or the built-in computeIfAbsent.
RBusarow added a commit that referenced this issue Jun 28, 2020
@RBusarow
- automatically cancel LifecycleCoroutineScope when lifecycle reaches…
36f291e 
… DESTROYED (fixes #135)

- automatically remove lifecycleScope extension property from cache when lifecycle reaches DESTROYED (fixes #136)
@RBusarow RBusarow mentioned this issue Jun 29, 2020
Merged
@RBusarow RBusarow linked a pull request Jun 29, 2020 that will close this issue
Lifecycle coroutine scope leak fixes #141
Merged
RBusarow added a commit that referenced this issue Jul 3, 2020
@RBusarow
Lifecycle coroutine scope leak fixes (#141)
093b60d 
* - automatically cancel LifecycleCoroutineScope when lifecycle reaches DESTROYED (fixes #135)
- automatically remove lifecycleScope extension property from cache when lifecycle reaches DESTROYED (fixes #136)

* dispatch-internal-test-android module

* update lifecycle handling

* update lifecycle handling

* update lifecycle handling

* convert LifecycleScopeExtensionTest from Kotest to JUnit5 to fix some weird recursive behavior

* convert LifecycleScopeExtensionTest from Kotest to JUnit5 to fix some weird recursive behavior

* remove workspace.xml backup which shouldn't have been added to git
@RBusarow RBusarow added this to the 1.0.0-beta04 milestone Jul 12, 2020
RBusarow added a commit that referenced this issue Nov 1, 2020
@RBusarow
Release/1.0.0 beta05 (#188)
c3ed048 
* update version to 1.0.0-beta04 (#137)

* Kotest 4.1.0 (#138)

* update Kotest to 4.1.0

* update Kotest to 4.1.0

* update Kotest to 4.1.0

* update Kotest to 4.1.0

* Revert "update Kotest to 4.1.0"

This reverts commit 833c838

* fix version parsing in DocsTasks

* add `release/*` matcher to ci.yml

* dispatch-internal-test-android module (#139)

* add hermit dependency (#140)

* add hermit dependency

* add Hermit to the dependency matchers in DocsTasks

* Lifecycle coroutine scope leak fixes (#141)

* - automatically cancel LifecycleCoroutineScope when lifecycle reaches DESTROYED (fixes #135)
- automatically remove lifecycleScope extension property from cache when lifecycle reaches DESTROYED (fixes #136)

* dispatch-internal-test-android module

* update lifecycle handling

* update lifecycle handling

* update lifecycle handling

* convert LifecycleScopeExtensionTest from Kotest to JUnit5 to fix some weird recursive behavior

* convert LifecycleScopeExtensionTest from Kotest to JUnit5 to fix some weird recursive behavior

* remove workspace.xml backup which shouldn't have been added to git

* update docs from main branch

* update change log (#142)

* Change LifecycleCoroutineScope argument to a CoroutineContext, add LifecycleCoroutineScopeFactory (#145)

* change LifecycleCoroutineScope argument to CoroutineContext

* MainImmediateProvidedContext -> MainImmediateContext

* MainImmediateProvidedContext -> MainImmediateContext

* update docs

* MainImmediateProvidedContext -> MainImmediateContext

* Misc cleanup (#146)

* LifecycleScopeFactory README example cleanup

* remove duplicate dependency declarations in android-lifecycle gradle config

* android-lifecycle sample annotation consolidation

* lifecycleScope extension sample rename

* MainImmediateCoroutineScope factory function formatting

* update docs

* - remove tabs (#147)

- consolidate capitalization for modules and kdoc
- add lifecycle-extensions
- dtekt -> detekt

* update Detekt to 1.10.0 (#148)

* update Knit to 0.1.4 (#149)

* coroutines 1.3.7 -> 1.3.8 (#150)

* Detekt cleanup (#151)

* Detekt cleanup

* add detekt to CI, remove Lint

* add dependency graph generator task

* DefaultDispatcherProvider (#153)

* add DefaultDispatcherProvider singleton holder (fixes #152)

* update docs for DefaultDispatcherProvider

* add resolution strategies for coroutines and dispatch (#155)

* cleanup of LifecycleCoroutineScope samples (#156)

* add ViewLifecycleCoroutineScope (#158)

* add ViewLifecycleCoroutineScope

* add ViewLifecycleCoroutineScope docs

* Lifecycle coroutine scope context parameter (#160)

* Add a CoroutineContext parameter to the Lifecycle launch and suspend functions

* add tests

* update docs

* update gradle to 6.6.1 (#163)

* dependency updates (#164)

- Kotlin 1.4.10
- coroutines 1.3.9
- Kotest 4.2.5

* sortDependencies task (#165)

* add and apply the sortDependencies task

* Update build.gradle.kts

* disable Jetifier (#166)

* update Robolectric to 4.4

* update Detekt to 1.14.1 (#168)

* Add the Dependency Analysis Android Gradle Plugin (#169)

* add the Dependency Analysis Plugin and apply its suggestions

* merge in the Detekt update

* restore the :dispatch:core dependency in :dispatch-android-viewmodel

* merge documentation/site changes from main branch

* add gradle doctor plugin (#171)

* add gradle enterprise plugin (#172)

* add task tree plugin (#173)

* junit test modules README cleanup

* update Kotest to 4.3.0 (#174)

* update to Gradle 6.7 and enable file system watching (#175)

* update androidx versions (#177)

* update kotlin plugin applications (#176)

* make testProvided's receiver a TestProvidedCoroutineScope (#180)

* update JUnit5 to 5.7.0 (#178)

* use currentCoroutineContext to resolve context inside a Flow (#182)

* update coroutines to 1.4.0 (#183)

* update Detekt to 1.4.2 https://github.com/detekt/detekt/releases/tag/v1.14.2 (#184)

* dependency updates (#185)

* DispatchLifecycleScope & DispatchViewModel (#186)

* rename LifecycleCoroutineScope to dispatchLifecycleScope.kt

* CoroutineViewModel -> DispatchViewModel

* CoroutineViewModel -> DispatchViewModel

* CoroutineViewModel -> DispatchViewModel

* remove docs/api from git

* update changelog for 1.0.0-beta05

* set min and target jvm to 8 (#187)

* add KDocs for deprecations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.0.0-beta04
Development

Successfully merging a pull request may close this issue.

Lifecycle coroutine scope leak fixes RBusarow/Dispatch
2 participants
@damianw @RBusarow