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

Clarify docs on actors #2459

Closed
Malinskiy opened this issue Dec 20, 2020 · 2 comments
Closed

Clarify docs on actors #2459

Malinskiy opened this issue Dec 20, 2020 · 2 comments
Labels

Comments

@Malinskiy
Copy link

Hello,

I have observed the following problem while using actors in my project:

Suppose we have multiple producers of events on different threads and one actor confined to a single thread executor. The actor has an internal state represented, for simplicity, as a Map<String, Int>. Once a particular message comes in, the state is checked for value (e.g. 0 or 1) and some functions are called depending on this state.

See sample test here for the scenario - https://gist.github.com/Malinskiy/8cbad94d8abce5dd90e37f0f4b932f5f. It takes random amount of time to observe the error but can fail with either -1 or 2 from my observations (although I think it may actually be any number there but these are closer because of the observation probability).

What I observe basically is that the sequence of executed operations (in this case check and increment) is non-deterministic.

private suspend fun check() {
    val i = map["key"]
    when(i) {
        0 -> increment()
        1 -> decrement()
        else -> throw RuntimeException("Mystery $i")
    }
}

private suspend fun increment() {
    delay(100)
    map["key"] = map["key"]!! + 1
    println("increment")
}

private suspend fun decrement() {
    delay(50)
    map["key"] = map["key"]!! - 1
}

The docs don't mention anything like this edge case. Can you please confirm that the order of execution for this is non-deterministic? In this particular case, the invoked suspend methods are not guaranteed to finish and this will break the An actor is a coroutine and a coroutine is executed sequentially, so confinement of the state to the specific coroutine works as a solution to the problem of shared mutable state.

Is the problem with the flow execution? That is, if the condition is executed in one suspend function, then the operating flow is given to processing another message that comes in (on the same thread) and it literally goes through the same flow of the condition, then proceeds to do 2 modifications at the same time?

As for possible solutions, I've thought of using the locks on this, but they would be reentrant and will not solve the issue. The only way I see to solve the problem is to not invoke the suspend functions (and hence don't give back the execution) which can get quite challenging to write everything in one method once the complexity of the actor grows.

Would love to hear back on this!
Thanks

@fvasco
Copy link
Contributor

fvasco commented Dec 21, 2020

An actor is a coroutine and a coroutine is executed sequentially, so confinement of the state to the specific coroutine works as a solution to the problem of shared mutable state.

So, please avoid to share the actor's state outside the coroutine, in you specific case the test procedure and all generated actor read and write map without any type of synchronization. Moreover you send the close message to an actor without await its completion.

You can easily fix this issue declaring the map variable inside the actor function and adding a Map parameter to check, increment and decrement.

@qwwdfsad
Copy link
Contributor

qwwdfsad commented Dec 21, 2020

@fvasco is indeed right, your state (map) is not confined to the single actor, but shared between all actors created in while(true) loop.
To avoid this class of errors in the future, I'd suggest you create a separate class that encapsulates both state and singe actor, basically emulating our future solution for #87

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants