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.

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

Expose atomic updates on MutableStateFlow #2720

Closed
lowasser opened this issue May 20, 2021 · 4 comments
Closed

Expose atomic updates on MutableStateFlow #2720

lowasser opened this issue May 20, 2021 · 4 comments

Comments

@lowasser
Copy link
Contributor

MutableStateFlow exposes compareAndSet, making it possible to do atomic updates on its value, but does not provide helpers to do so correctly. The StateFlow documentation even shows an example method

fun inc() {
  _counter.value++
}

which is not safe to be used concurrently.

Correct atomic update methods like those in atomicfu are not hard to build yourself if you know what you're doing, but providing them directly -- and showing their correct use in the documentation examples -- seems like a good idea.

(So, for example, I'd expect to see MutableStateFlow<T>.updateAndGet(updater: (T) -> T): T. Number-specific helpers like addAndGet or incrementAndGet would be nice to have, but the basics would suffice.)

lowasser added a commit to lowasser/kotlinx.coroutines that referenced this issue May 20, 2021
qwwdfsad added a commit that referenced this issue May 25, 2021
…ableStateFlow (#2729)

* Add update, updateAndGet, and getAndUpdate extension functions to MutableStateFlow (#2720).

Fixes #2720

Co-authored-by: Louis Wasserman <lowasser@google.com>
@lowasser
Copy link
Contributor Author

Looks closed to me!

@joffrey-bion
Copy link
Contributor

joffrey-bion commented Aug 17, 2021

Sorry to be late to the party, but I find it strange that this finally made it to the library given the explanations I got when I first asked for it in #2392.

I believe the drawbacks of the looping implementation should be noted in the doc of the new helpers, because for complex objects where equals is not trivial, it might not be desirable to perform repeated calls to compareAndSet (as pointed out in the initial discussion).

@elizarov
Copy link
Contributor

Exposing the locking variant (without CAS) is extremely fragile. The resulting design would suffer from a host of hard-to-understand issues similar to issues Java's ConcurrentMap.computeIfAbsent suffers from. CAS-based approach seems to be a reasonably good compromise as it performs well as long as you don't have too many concurrent mutation attempts. If your particular application needs lock-based updates, then you can always build it on top of the StateFlow's API yourself using a separate Mutex. The converse is not true. If we don't provide you with CAS-based updates, then you cannot build your own.

@joffrey-bion
Copy link
Contributor

Thanks a lot for the fast response. I agree with the problems you mentioned about the lock-based version, so yeah I should actually not be surprised that the CAS version was introduced (I'm actually happy about it).

I just believe it might be worth adding to the doc that this could repeatedly call compareAndSet (in addition to repeatedly call the transform function), because both of those could be expensive depending on the state's type.

pablobaxter pushed a commit to pablobaxter/kotlinx.coroutines that referenced this issue Sep 14, 2022
…ableStateFlow (Kotlin#2729)

* Add update, updateAndGet, and getAndUpdate extension functions to MutableStateFlow (Kotlin#2720).

Fixes Kotlin#2720

Co-authored-by: Louis Wasserman <lowasser@google.com>
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

No branches or pull requests

3 participants