Repository: freeletics/FlowRedux (kotlin)

Issue:
[Question] Dead lock in dispatching multiple actions to ORDERED OnAction statement
I made a simple snippst to reproduce my issue on 1.2.0:

https://gist.github.com/xiaozhikang0916/6f4cf768e547d48a30479a9c13547ddb

It comes to a dead locking situation and the state machine becomes unable to handle any up coming action.

And I found some ways to make it works:

1. Remove all the `executionPolicy = ExecutionPolicy.ORDERED` statement in the statemachine dsl, or
2. Dispatch ActionA before ActionB, to meet the order of OnAction statement in the dsl.

I wonder why the order of statement matters here.

This code works fine in 1.1.0

Pull request:
The reduxStore has a lock so that [state changes](https://github.com/freeletics/FlowRedux/blob/main/flowredux/src/commonMain/kotlin/com/freeletics/flowredux/FlowRedux.kt#L40-L50)) and [handling actions](
https://github.com/freeletics/FlowRedux/blob/main/flowredux/src/commonMain/kotlin/com/freeletics/flowredux/FlowRedux.kt#L56-L60) don't happen at the same time which could result in actions being processed when we stop a side effect or actions being dropped while changing to the new state where they would actually be handled. The deadlock then happens because the state change from the first of the 2 actions waits at the lock entry while the second action is being processed. However the processing of the second action never finishes because it suspends when trying to add the second state change to `stateChanges` and that will only be possible once the first state change was completed.

To fix this I've changed the `stateChanges` channel from `RENDEZVOUS` to `UNLIMITED`. This way new state changes can be added to it while the previous is still in the reducer and waiting for the lock. Alternatively it would have been possible to change the internal action channels to buffer actions however that would introduce more perceived randomness to the action processing (based on how coroutines get scheduled) so I think it's better to have it in just one place.

Theoretically we could expose an option on the state machine level to decide whether `stateChanges` should be `UNLIMITED` or `BUFFERED` with a `DROP_OLDEST` policy in case the buffer overflows, but I'd only do that if someone actually runs into the need for this.

Closes #608

Patch:
diff --git a/flowredux/src/commonMain/kotlin/com/freeletics/flowredux/FlowRedux.kt b/flowredux/src/commonMain/kotlin/com/freeletics/flowredux/FlowRedux.kt
index 39e5fe5e..ed42fb3f 100644
--- a/flowredux/src/commonMain/kotlin/com/freeletics/flowredux/FlowRedux.kt
+++ b/flowredux/src/commonMain/kotlin/com/freeletics/flowredux/FlowRedux.kt
@@ -20,7 +20,7 @@ internal fun <A : Any, S : Any> Flow<A>.reduxStore(
     var currentState: S = initialStateSupplier()
     val getState: GetState<S> = { currentState }
 
-    val stateChanges = Channel<ChangedState<S>>()
+    val stateChanges = Channel<ChangedState<S>>(Channel.UNLIMITED)
     val sideEffects = sideEffectBuilders.map { ManagedSideEffect(it, this@channelFlow, getState, stateChanges) }
 
     // Emit the initial state


Test patch:
diff --git a/flowredux/src/commonTest/kotlin/com/freeletics/flowredux/sideeffects/OnActionTest.kt b/flowredux/src/commonTest/kotlin/com/freeletics/flowredux/sideeffects/OnActionTest.kt
index f061a9fa..a996e875 100644
--- a/flowredux/src/commonTest/kotlin/com/freeletics/flowredux/sideeffects/OnActionTest.kt
+++ b/flowredux/src/commonTest/kotlin/com/freeletics/flowredux/sideeffects/OnActionTest.kt
@@ -2,16 +2,21 @@ package com.freeletics.flowredux.sideeffects
 
 import app.cash.turbine.awaitItem
 import app.cash.turbine.test
+import app.cash.turbine.turbineScope
 import com.freeletics.flowredux.StateMachine
 import com.freeletics.flowredux.TestAction
 import com.freeletics.flowredux.TestState
+import com.freeletics.flowredux.dsl.ExecutionPolicy
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertIs
 import kotlin.test.assertTrue
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.awaitCancellation
 import kotlinx.coroutines.channels.Channel
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.test.runTest
 
 @OptIn(ExperimentalCoroutinesApi::class)
@@ -74,4 +79,32 @@ internal class OnActionTest {
             assertEquals(TestState.S2, awaitItem())
         }
     }
+
+    @Test
+    fun onActionOrdered() = runTest {
+        turbineScope {
+            val sm = StateMachine(TestState.GenericNullableState(null, null)) {
+                inState<TestState.GenericNullableState> {
+                    on<TestAction.A1>(executionPolicy = ExecutionPolicy.ORDERED) { _, state ->
+                        state.mutate { copy(aString = "1") }
+                    }
+
+                    on<TestAction.A2>(executionPolicy = ExecutionPolicy.ORDERED) { _, state ->
+                        state.mutate { copy(anInt = 2) }
+                    }
+                }
+            }
+
+            val scope = CoroutineScope(context = Dispatchers.Unconfined)
+            val turbine = sm.state.testIn(scope)
+            scope.launch {
+                sm.dispatch(TestAction.A2)
+                sm.dispatch(TestAction.A1)
+            }
+
+            assertEquals(TestState.GenericNullableState(null, null), turbine.awaitItem())
+            assertEquals(TestState.GenericNullableState("1", null), turbine.awaitItem())
+            assertEquals(TestState.GenericNullableState("1", 2), turbine.awaitItem())
+        } 
+    }
 }