Skip to content

Commit b8bca90

Browse files
authored
fix(android): use ConcurrentHashMap for listeners to prevent ConcurrentModificationException (#286)
`onChanged` iterates `listeners.values` on a `Dispatchers.Default` thread (the flow-collection coroutine), while `addListener`/`removeListener` mutate the same map from the JS thread. A plain `LinkedHashMap` fails fast with `ConcurrentModificationException` when this overlap occurs. `ConcurrentHashMap` iterates safely with concurrent mutations — no CME, no locks needed at call sites.
1 parent 42e7d76 commit b8bca90

5 files changed

Lines changed: 55 additions & 7 deletions

File tree

.github/workflows/ci.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,9 @@ jobs:
507507
run: |
508508
cd example
509509
bundle install
510+
# Drop the experimental-generated lock so the legacy backend resolves
511+
# fresh (avoids a stale fast_float podspec checksum mismatch).
512+
rm -f ios/Podfile.lock ios/Pods/Manifest.lock
510513
USE_RIVE_LEGACY=1 bundle exec pod install --project-directory=ios
511514
512515
- name: Save cocoapods cache

android/src/main/java/com/margelo/nitro/rive/BaseHybridViewModelPropertyImpl.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package com.margelo.nitro.rive
33
import androidx.annotation.Keep
44
import com.facebook.proguard.annotations.DoNotStrip
55
import kotlinx.coroutines.CoroutineScope
6+
import kotlinx.coroutines.CoroutineStart
67
import kotlinx.coroutines.Dispatchers
78
import kotlinx.coroutines.Job
89
import kotlinx.coroutines.cancel
@@ -11,20 +12,21 @@ import kotlinx.coroutines.flow.Flow
1112
import kotlinx.coroutines.flow.drop
1213
import java.lang.ref.WeakReference
1314
import java.util.UUID
15+
import java.util.concurrent.ConcurrentHashMap
1416

1517
@Keep
1618
@DoNotStrip
1719
class BaseHybridViewModelPropertyImpl<T> : BaseHybridViewModelProperty<T> {
1820
override var scope: CoroutineScope? = null
1921
override var job: Job? = null
20-
override val listeners = mutableMapOf<String, (T) -> Unit>()
22+
override val listeners = ConcurrentHashMap<String, (T) -> Unit>()
2123

2224
override fun ensureValueListenerJob(valueFlow: Flow<T>, drop: Int) {
2325
if (scope == null) {
2426
scope = CoroutineScope(Dispatchers.Default)
2527
}
2628
if (job == null) {
27-
job = scope?.launch {
29+
job = scope?.launch(start = CoroutineStart.UNDISPATCHED) {
2830
valueFlow.drop(drop).collect { value ->
2931
onChanged(value)
3032
}

android/src/new/java/com/margelo/nitro/rive/HybridViewModelTriggerProperty.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ class HybridViewModelTriggerProperty(
1818

1919
override fun addListener(onChanged: () -> Unit): () -> Unit {
2020
val remover = addListenerInternal { _ -> onChanged() }
21-
ensureValueListenerJob(instance.getTriggerFlow(path), 1)
21+
// Triggers use drop=0: getTriggerFlow has replay=0 and emits nothing on
22+
// subscription, so there is no initial value to skip.
23+
ensureValueListenerJob(instance.getTriggerFlow(path), 0)
2224
return remover
2325
}
2426
}

android/src/new/java/com/rive/RiveReactNativeView.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,12 @@ class RiveReactNativeView(context: ThemedReactContext) : FrameLayout(context) {
132132
worker.advanceStateMachine(sm, deltaTime)
133133
worker.draw(art, sm, rs, activeFit)
134134
frameCount++
135+
// Signal readiness only once the state machine is actually running
136+
// (surface created + first frame drawn), so callers awaiting
137+
// awaitViewReady() before firing triggers don't fire too early.
138+
if (frameCount == 1L) {
139+
viewReadyDeferred.complete(true)
140+
}
135141
} catch (e: Exception) {
136142
Log.e(TAG, "Render loop error", e)
137143
}
@@ -203,7 +209,6 @@ class RiveReactNativeView(context: ThemedReactContext) : FrameLayout(context) {
203209
applyDataBinding(config.bindData, config.riveFile)
204210
}
205211

206-
viewReadyDeferred.complete(true)
207212
}
208213

209214
private fun resizeArtboardIfLayout() {

example/__tests__/use-rive-trigger.harness.tsx

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
RiveView,
1515
useRiveTrigger,
1616
type RiveFile,
17+
type RiveViewRef,
1718
} from '@rive-app/react-native';
1819
import type { ViewModelInstance } from '@rive-app/react-native';
1920

@@ -39,10 +40,33 @@ type TriggerContext = {
3940
triggerFn: (() => void) | null;
4041
error: Error | null;
4142
renderCount: number;
43+
ref: RiveViewRef | null;
4244
};
4345

4446
function createTriggerContext(): TriggerContext {
45-
return { triggerCount: 0, triggerFn: null, error: null, renderCount: 0 };
47+
return {
48+
triggerCount: 0,
49+
triggerFn: null,
50+
error: null,
51+
renderCount: 0,
52+
ref: null,
53+
};
54+
}
55+
56+
/**
57+
* Waits until the Rive view's state machine is live before firing triggers.
58+
* A fireTrigger() issued before the surface/state-machine is advancing is
59+
* dropped (the instance's trigger flow never emits it), so tests must wait for
60+
* readiness first instead of firing synchronously on mount.
61+
*/
62+
async function waitForViewReady(context: TriggerContext): Promise<void> {
63+
await waitFor(
64+
() => {
65+
expect(context.ref).not.toBeNull();
66+
},
67+
{ timeout: 5000 }
68+
);
69+
await context.ref!.awaitViewReady();
4670
}
4771

4872
// ─── Test component: stable callback ───────────────────────────────
@@ -74,6 +98,11 @@ function StableTriggerComponent({
7498
return (
7599
<View style={{ width: 200, height: 200 }}>
76100
<RiveView
101+
hybridRef={{
102+
f: (ref: RiveViewRef | null) => {
103+
context.ref = ref;
104+
},
105+
}}
77106
file={file}
78107
fit={Fit.Contain}
79108
dataBind={instance}
@@ -123,6 +152,11 @@ function UnstableTriggerComponent({
123152
return (
124153
<View style={{ width: 200, height: 200 }}>
125154
<RiveView
155+
hybridRef={{
156+
f: (ref: RiveViewRef | null) => {
157+
context.ref = ref;
158+
},
159+
}}
126160
file={file}
127161
fit={Fit.Contain}
128162
dataBind={instance}
@@ -156,8 +190,9 @@ describe('useRiveTrigger hook', () => {
156190

157191
expect(context.error).toBeNull();
158192

159-
// Fire trigger and wait for it — pollChanges() runs on frame ticks,
160-
// so we wait for each trigger individually to avoid coalescing.
193+
// Wait for the state machine to be live before firing — an early
194+
// fireTrigger() (before the surface/state-machine is advancing) is dropped.
195+
await waitForViewReady(context);
161196
context.triggerFn!();
162197

163198
await waitFor(
@@ -200,6 +235,7 @@ describe('useRiveTrigger hook', () => {
200235
expect(context.error).toBeNull();
201236

202237
// Fire trigger AFTER the re-render burst — before the fix, this was lost
238+
await waitForViewReady(context);
203239
context.triggerFn!();
204240

205241
await waitFor(

0 commit comments

Comments
 (0)