Fixed
Status Update
Comments
jb...@google.com <jb...@google.com>
il...@google.com <il...@google.com>
mg...@google.com <mg...@google.com>
ap...@google.com <ap...@google.com> #2
Project: platform/frameworks/support
Branch: androidx-main
commit b261395469b81e8fd3b07e913dfaf0766b4b6410
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Fri Mar 08 19:05:24 2024
Add lock to `viewModelScope`
RelNote: "Accessing the `viewModelScope` of a `ViewModel` is now thread safe."
Test: ViewModelTest
Bug: 322407038
Change-Id: If4766608206f1354b03463b2771b7a369a0b537f
M lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/ViewModel.kt
https://android-review.googlesource.com/2996023
Branch: androidx-main
commit b261395469b81e8fd3b07e913dfaf0766b4b6410
Author: Marcello Galhardo <mgalhardo@google.com>
Date: Fri Mar 08 19:05:24 2024
Add lock to `viewModelScope`
RelNote: "Accessing the `viewModelScope` of a `ViewModel` is now thread safe."
Test: ViewModelTest
Bug: 322407038
Change-Id: If4766608206f1354b03463b2771b7a369a0b537f
M lifecycle/lifecycle-viewmodel/src/commonMain/kotlin/androidx/lifecycle/ViewModel.kt
mg...@google.com <mg...@google.com>
pr...@google.com <pr...@google.com> #3
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.lifecycle:lifecycle-viewmodel:2.8.0-alpha03
androidx.lifecycle:lifecycle-viewmodel-android:2.8.0-alpha03
Description
Version used:
2.8.0-alpha01
In the latest alpha version, there's now the ability to retrieve
Closeable
objects previously added to aViewModel
, which is great.I did notice though that the (internal)
setTagIfAbsent
method has been removed andViewModel.viewModelScope
(which previously usedsetTagIfAbsent
) now usesgetCloseable
andaddCloseable
separately.I'm thinking this might be problematic as the operation appears to no longer be atomic? E.g. the
viewModelScope
may be saved and then a new one saved again, which will overwrite the first one in theMap
and hence whatever job(s) it had launched would no longer be cancelled with theViewModel
.I assume that the issue of concurrency is important here given the
synchronized
blocks, and that other scopes seem to have pretty stringent measures (e.g.Lifecycle.coroutineScope
). MaybeCollections.synchronizedMap
andCollections.synchronizedSet
could also be helpful?Aside from concurrency issues, the new
addCloseable
method doesn't actually take into consideration whether aCloseable
has already been added with the same key, so there is plenty of potential for accidental overwrites, which would likely cause leaks when the the original overwrittenCloseable
disappears from theMap
.Regardless of the above, it would be nice to have something along the lines of
putIfAbsent
and/orcomputeIfAbsent
in the publicViewModel
API as part of this new feature, alongside the standardaddCloseable
andgetCloseable
.