Skip to content

Commit

Permalink
Make formula internals thread safe.
Browse files Browse the repository at this point in the history
  • Loading branch information
Laimiux committed Jan 21, 2024
1 parent 8681b24 commit b3abe8a
Show file tree
Hide file tree
Showing 11 changed files with 286 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.instacart.formula.FormulaPlugins
import com.instacart.formula.FormulaRuntime
import com.instacart.formula.IFormula
import com.instacart.formula.Inspector
import com.instacart.formula.internal.SynchronizedEventQueue
import com.instacart.formula.internal.ThreadChecker
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.channels.awaitClose
Expand All @@ -25,15 +26,17 @@ object FlowRuntime {
): Flow<Output> {
val threadChecker = ThreadChecker(formula)
return callbackFlow<Output> {
threadChecker.check("Need to subscribe on main thread.")
// threadChecker.check("Need to subscribe on main thread.")

val mergedInspector = FormulaPlugins.inspector(
type = formula.type(),
local = inspector,
)

val queue = SynchronizedEventQueue()
val runtimeFactory = {
FormulaRuntime(
queue = queue,
threadChecker = threadChecker,
formula = formula,
onOutput = this::trySendBlocking,
Expand All @@ -46,17 +49,21 @@ object FlowRuntime {
var runtime = runtimeFactory()

input.onEach { input ->
threadChecker.check("Input arrived on a wrong thread.")
if (!runtime.isKeyValid(input)) {
runtime.terminate()
runtime = runtimeFactory()
// threadChecker.check("Input arrived on a wrong thread.")
queue.postUpdate {
if (!runtime.isKeyValid(input)) {
runtime.terminate()
runtime = runtimeFactory()
}
runtime.onInput(input)
}
runtime.onInput(input)
}.launchIn(this)

awaitClose {
threadChecker.check("Need to unsubscribe on the main thread.")
runtime.terminate()
// threadChecker.check("Need to unsubscribe on the main thread.")
queue.postUpdate {
runtime.terminate()
}
}
}.distinctUntilChanged()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.instacart.formula.FormulaPlugins
import com.instacart.formula.FormulaRuntime
import com.instacart.formula.IFormula
import com.instacart.formula.Inspector
import com.instacart.formula.internal.SynchronizedEventQueue
import com.instacart.formula.internal.ThreadChecker
import io.reactivex.rxjava3.core.Observable
import io.reactivex.rxjava3.disposables.CompositeDisposable
Expand All @@ -22,8 +23,10 @@ object RxJavaRuntime {
type = formula.type(),
local = inspector,
)
val queue = SynchronizedEventQueue()
val runtimeFactory = {
FormulaRuntime(
queue = queue,
threadChecker = threadChecker,
formula = formula,
onOutput = emitter::onNext,
Expand All @@ -33,23 +36,27 @@ object RxJavaRuntime {
)
}

threadChecker.check("Need to subscribe on main thread.")
// threadChecker.check("Need to subscribe on main thread.")

var runtime = runtimeFactory()

val disposables = CompositeDisposable()
disposables.add(input.subscribe({ input ->
threadChecker.check("Input arrived on a wrong thread.")
if (!runtime.isKeyValid(input)) {
runtime.terminate()
runtime = runtimeFactory()
// threadChecker.check("Input arrived on a wrong thread.")
queue.postUpdate {
if (!runtime.isKeyValid(input)) {
runtime.terminate()
runtime = runtimeFactory()
}
runtime.onInput(input)
}
runtime.onInput(input)
}, emitter::onError))

val runnable = Runnable {
threadChecker.check("Need to unsubscribe on the main thread.")
runtime.terminate()
// threadChecker.check("Need to unsubscribe on the main thread.")
queue.postUpdate {
runtime.terminate()
}
}
disposables.add(FormulaDisposableHelper.fromRunnable(runnable))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class TestFormulaObserver<Input : Any, Output : Any, FormulaT : IFormula<Input,
private val delegate: FormulaTestDelegate<Input, Output, FormulaT>,
) {

private var started: Boolean = false
@Volatile private var started: Boolean = false

val formula: FormulaT = delegate.formula

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ package com.instacart.formula
import com.instacart.formula.internal.FormulaManager
import com.instacart.formula.internal.FormulaManagerImpl
import com.instacart.formula.internal.ManagerDelegate
import com.instacart.formula.internal.SynchronizedEventQueue
import com.instacart.formula.internal.ThreadChecker
import java.util.LinkedList
import java.util.concurrent.atomic.AtomicReference

/**
* Takes a [Formula] and creates an Observable<Output> from it.
*/
class FormulaRuntime<Input : Any, Output : Any>(
private val queue: SynchronizedEventQueue,
private val threadChecker: ThreadChecker,
private val formula: IFormula<Input, Output>,
private val onOutput: (Output) -> Unit,
Expand Down Expand Up @@ -61,6 +64,7 @@ class FormulaRuntime<Input : Any, Output : Any>(

if (initialization) {
manager = FormulaManagerImpl(
queue = queue,
delegate = this,
formula = implementation,
initialInput = input,
Expand Down Expand Up @@ -95,7 +99,7 @@ class FormulaRuntime<Input : Any, Output : Any>(
}

override fun onPostTransition(effects: Effects?, evaluate: Boolean) {
threadChecker.check("Only thread that created it can post transition result")
// threadChecker.check("Only thread that created it can post transition result")

effects?.let {
globalEffectQueue.addLast(effects)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ internal class ChildrenManager(
val childFormulaHolder = children.findOrInit(key) {
val implementation = formula.implementation()
FormulaManagerImpl(
queue = delegate.queue,
delegate = delegate,
formula = implementation,
initialInput = input,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import kotlin.reflect.KClass
* a state change, it will rerun [Formula.evaluate].
*/
internal class FormulaManagerImpl<Input, State, Output>(
val queue: SynchronizedEventQueue,
private val delegate: ManagerDelegate,
private val formula: Formula<Input, State, Output>,
initialInput: Input,
Expand Down Expand Up @@ -259,6 +260,7 @@ internal class FormulaManagerImpl<Input, State, Output>(
}

fun onPendingTransition(transition: DeferredTransition<*, *, *>) {
// TODO: we should pass it to the runtime to ensure proper synchronization
if (terminated) {
transition.execute()
} else if (isRunning) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ internal class ListenerImpl<Input, State, EventT>(internal var key: Any) : Liste
// TODO: log if null listener (it might be due to formula removal or due to callback removal)
val manager = manager ?: return

val deferredTransition = DeferredTransition(this, transition, event)
manager.onPendingTransition(deferredTransition)
manager.queue.postUpdate {
val deferredTransition = DeferredTransition(this, transition, event)
manager.onPendingTransition(deferredTransition)
}
}

fun disable() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package com.instacart.formula.internal

import java.util.concurrent.ConcurrentLinkedQueue
import java.util.concurrent.atomic.AtomicReference

/**
* A non-blocking event queue that processes formula updates.
*/
class SynchronizedEventQueue {
private val threadRunning = AtomicReference<Thread>()
private val concurrentLinkedQueue = ConcurrentLinkedQueue<() -> Unit>()

/**
* All top-level formula interactions that trigger formula side-effects are posted here
* to make sure that they are executed one at a time. If there is a thread currently running
* formula, we hand the update to that thread for processing. The following
* root formula events are propagated via this queue:
* - Input change
* - Individual formula transitions
* - Termination
*
* Implementation works by having a concurrent queue and checking:
* - If queue is idle, execute current update and try to process other queue entries
* - If queue is running by the same thread, we execute current update and let other
* updates be handled by existing processing loop.
* - If queue is running by a different thread, add to the queue and see if we need to
* take over the processing.
*/
fun postUpdate(runnable: () -> Unit) {
val currentThread = Thread.currentThread()
val owner = threadRunning.get()
if (owner == currentThread) {
// Since we are on the same thread, just execute the event (no need to grab ownership)
runnable()
} else if (owner == null) {
if (threadRunning.compareAndSet(null, currentThread)) {
// The queue is idle, we first execute our own event and then move to the queue
runnable()
threadRunning.set(null)

tryToProcessQueueIfNeeded(currentThread)
} else {
concurrentLinkedQueue.add(runnable)
tryToProcessQueueIfNeeded(currentThread)
}
} else {
concurrentLinkedQueue.add(runnable)
tryToProcessQueueIfNeeded(currentThread)
}
}

private fun tryToProcessQueueIfNeeded(currentThread: Thread) {
while (true) {
// First, we peek to see if there is a value to process.
val peekUpdate = concurrentLinkedQueue.peek()
if (peekUpdate != null) {
// If there is a value to process, we check if we should process it.
if (threadRunning.compareAndSet(null, currentThread)) {
// We successfully set ourselves as the running thread
// We poll the queue to get the latest value (it could have changed). It
// also removes the value from the queue.
val actualUpdate = concurrentLinkedQueue.poll()
actualUpdate?.invoke()
threadRunning.set(null)
} else {
// Some other thread is running, let that thread execute the update.
return
}
} else {
return
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import com.instacart.formula.subjects.KeyFormula
import com.instacart.formula.subjects.KeyUsingListFormula
import com.instacart.formula.subjects.MessageFormula
import com.instacart.formula.subjects.MixingCallbackUseWithKeyUse
import com.instacart.formula.subjects.MultiThreadRobot
import com.instacart.formula.subjects.MultipleChildEvents
import com.instacart.formula.subjects.NestedCallbackCallRobot
import com.instacart.formula.subjects.NestedChildTransitionAfterNoEvaluationPass
Expand Down Expand Up @@ -62,6 +63,7 @@ import com.instacart.formula.subjects.TestKey
import com.instacart.formula.subjects.TransitionAfterNoEvaluationPass
import com.instacart.formula.subjects.UseInputFormula
import com.instacart.formula.subjects.ReusableFunctionCreatesUniqueListeners
import com.instacart.formula.subjects.SleepFormula
import com.instacart.formula.subjects.UniqueListenersWithinLoop
import com.instacart.formula.subjects.UsingKeyToScopeCallbacksWithinAnotherFunction
import com.instacart.formula.subjects.UsingKeyToScopeChildFormula
Expand All @@ -83,6 +85,9 @@ import org.junit.rules.RuleChain
import org.junit.rules.TestName
import org.junit.runner.RunWith
import org.junit.runners.Parameterized
import java.util.concurrent.CountDownLatch
import java.util.concurrent.Executors
import java.util.concurrent.ThreadFactory
import java.util.concurrent.TimeUnit
import kotlin.reflect.KClass

Expand Down Expand Up @@ -646,7 +651,9 @@ class FormulaRuntimeTest(val runtime: TestableRuntime, val name: String) {
}

val observer = runtime.test(formula, Unit)
bgAction.latch.await(50, TimeUnit.MILLISECONDS)
if (!bgAction.latch.await(50, TimeUnit.MILLISECONDS)) {
throw IllegalStateException("Timeout")
}
assertThat(bgAction.errors.values().firstOrNull()?.message).contains(
"com.instacart.formula.subjects.OnlyUpdateFormula - Only thread that created it can post transition result Expected:"
)
Expand Down Expand Up @@ -1294,6 +1301,24 @@ class FormulaRuntimeTest(val runtime: TestableRuntime, val name: String) {
.assertValue(0)
}

@Test
fun `formula multi thread handoff`() {
with(MultiThreadRobot(runtime)) {
threadA(50)
threadB(10)
awaitCompletion()
threadB(10)

awaitEvents(
SleepFormula.SleepEvent(50, "thread-a"),
// First thread-b event is handed-off to thread-a
SleepFormula.SleepEvent(10, "thread-a"),
// Second thread-b event is handled by thread-b
SleepFormula.SleepEvent(10, "thread-b")
)
}
}

@Test
fun `inspector events`() {
val globalInspector = TestInspector()
Expand Down
Loading

0 comments on commit b3abe8a

Please sign in to comment.