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
Yigit, do you have time to fix it?
reemission of the same liveData is racy
reemission of the same liveData is racy
mg...@google.com <mg...@google.com>
pr...@google.com <pr...@google.com> #3
yea i'll take it.
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
.