fix: make Rive data binding thread-safe on iOS & Android (#297) by mfazekas · Pull Request #298 · rive-app/rive-nitro-react-native

mfazekas · 2026-06-18T13:02:22Z

Summary

Fixes the occasional crashes in #297. The Rive data-binding objects aren't thread-safe and must be driven from a single thread, but the Nitro layer accessed them from several threads with no synchronization:

iOS — the Rive SDK plus the Obj-C RiveDataBindingViewModelInstance / C++ ViewModelInstanceRuntime caches are main-thread-only, yet they were touched from the JS thread (sync accessors), the Swift cooperative pool (every Promise.async), and the main render loop. Concurrent access corrupts those caches → the three crash stacks in the issue.
Android — BaseHybridViewModelPropertyImpl iterated its listener map on a Dispatchers.Default flow collector while addListener/removeListener mutated it from the JS thread → ConcurrentModificationException (this is what crashed the test-harness-android CI job).

The fix

iOS — route all data-binding access through the main thread:

MainThread.run — runs inline on main, else hops via DispatchQueue.main.sync.
Promise.onMain — dispatches work to the main queue asynchronously (non-blocking) and returns a Promise that resolves once the work completes on main. Replaces Promise.async, which ran closures on the Swift cooperative pool off-main.
Applied to HybridViewModelInstance, HybridViewModel, every HybridViewModel*Property, the property listener helper, and the deprecated imperative RiveView input/text-run methods (set/getNumberInputValue, set/getBooleanInputValue, triggerInput, set/getTextRunValue).
Bumps react-native-nitro-modules to 0.35.9, which fixes a Swift-level Promise listener race (fix: Fix Promise<void> was destroyed! race condition by synchronizing Swift Promise operations mrousavy/nitro#1338). Full C++ object-bridging coverage lands in fix(core): synchronize C++ Promise<T> state-inspection accessors mrousavy/nitro#1398.

Android — BaseHybridViewModelPropertyImpl now uses a ConcurrentHashMap and iterates a toList() snapshot in onChanged, so dispatch is safe even if a callback adds/removes a listener mid-iteration.

Verification (Thread Sanitizer)

Built the iOS example with -enableThreadSanitizer YES and ran a concurrent data-binding stress (viewModelAsync + synchronous property access on the same instance). TSan reports data races on the unfixed build and none after the fix; under heavy load the unfixed build crashes, the fixed one doesn't.

On Android, the harness test hits the ConcurrentModificationException crash before the fix and passes after.

Tests

react-native-harness tests — they always pass, the sanitizer is the real assertion (unfixed runtime → race reported / crash; fixed → clean):

example/src/__tests__/issue297.databinding.test.ts — viewModelAsync (off-main) concurrent with synchronous property access. Raise CONCURRENCY/ROUNDS to reproduce the hard crash (what took down the Android CI job).
example/src/__tests__/issue297.hooks.test.tsx — the same bug via the public use* hooks: a live RiveView advances the state machine on the render thread while useRiveNumber churns the property (writes + addListener) on the JS thread.

example/src/reproducers/Issue297ThreadRace.tsx is a manual in-app reproducer.

Running the harness tests under Thread Sanitizer (iOS)

TSan is a compile flag, and the harness only drives a prebuilt app (it has no build step), so it can't be a harness option — it's the normal build plus one xcodebuild flag. yarn ios builds and installs the app, then the harness drives it:

cd example
yarn ios --no-packager --extra-params "-enableThreadSanitizer YES"
yarn test:harness:ios --testPathPattern issue297

--no-packager stops run-ios from starting Metro (the harness starts its own). The harness auto-targets the booted simulator (see rn-harness.config.mjs). TSan reports land in /tmp/tsan_harness.<pid>; grep for WARNING: ThreadSanitizer: data race — present on the unfixed runtime, absent after the fix.

Prefer an explicit build + install (what CI does)? Note build:ios needs --buildFolder to land in ios/build:

yarn build:ios --buildFolder build --extra-params "-enableThreadSanitizer YES"
xcrun simctl install booted ios/build/Build/Products/Debug-iphonesimulator/RiveExample.app
yarn test:harness:ios --testPathPattern issue297

Or enable Thread Sanitizer in the Xcode scheme (Run → Diagnostics) and ⌘R once to build+install.

The Rive iOS runtime (RiveViewModel/RiveView, the Obj-C RiveDataBindingViewModelInstance caches, and the C++ ViewModelInstanceRuntime maps) is not thread-safe and must be driven from the main thread, where the state machine advances and property listeners fire. The Nitro layer accessed it from three threads with no synchronization: - main render thread (state machine advance / afterUpdate reconfigure) - JS thread (synchronous property accessors) - Swift cooperative pool (every `Promise.async` block) Concurrent access corrupts those caches, producing the crashes in #297 (NSMutableDictionary mutation, unordered_map free-of-unallocated, artboard instancing). Thread Sanitizer flagged ~25 data races at the data-binding boundary. Route all data-binding access through the main thread: - add `MainThread.run` (runs inline on main, hops via `DispatchQueue.main.sync` otherwise) and `Promise.onMain` (does the work on main and returns an already-resolved Promise, so Nitro's `.then` never races a background resolve) - apply to HybridViewModelInstance, HybridViewModel, every HybridViewModel*Property, and the property listener helper After: TSan reports 0 races under the same stress; data binding still renders.

- example/src/__tests__/issue297.databinding.test.ts: on-device harness test that drives `viewModelAsync` (resolved off-main) concurrently with synchronous property access on the same ViewModelInstance. It stays green; built with Thread Sanitizer it reports the data race (unfixed) or is clean (fixed). Turn CONCURRENCY/ROUNDS up to reproduce the hard crash. - rn-harness.config.mjs: HARNESS_TSAN-gated TSAN_OPTIONS via appLaunchOptions, and HARNESS_METRO_PORT override (defaults unchanged). - Issue297ThreadRace.tsx: manual reproducer screen (same stress in-app). - gitignore the .harness/ build cache.

…race

…no-deprecated in repro)

Renders a live RiveView (state machine on the render thread) and churns the same property from the JS thread through useRiveNumber (writes + addListener via remounts). Under a TSan build it flags the listener race on the unfixed runtime, clean on the fixed one.

…ad (#297) setNumberInputValue/getNumberInputValue, setBooleanInputValue/getBooleanInputValue, triggerInput, and setTextRunValue/getTextRunValue touch the artboard/state machine from the JS thread. Funnel them through MainThread.run, same as the data-binding API.

…modification (#297) onChanged() iterates the listener map on a Dispatchers.Default flow collector while addListener/removeListener mutate it from the JS/native thread, throwing ConcurrentModificationException and crashing the app (the Android analogue of the data-binding race fixed on iOS). Use ConcurrentHashMap and iterate over a toList() snapshot so dispatch is safe even if a callback adds/removes a listener mid-iteration.

…under TSan in one step The harness only drives a prebuilt app, so Thread Sanitizer (a compile flag) must be baked into the app rather than passed to the harness. This script builds the example with -enableThreadSanitizer YES, installs it on the booted simulator, and runs the harness against it.

TSAN_OPTIONS is only read by the TSan runtime and ignored on a non-TSan build, so the HARNESS_TSAN gate was pointless. Always set it; running under TSan is now just 'yarn test:harness:ios' once the app is built with -enableThreadSanitizer YES.

Auto-detect the booted simulator in rn-harness.config.mjs (override with DEVICE_MODEL / IOS_VERSION; falls back to the previous fixed default). Now 'yarn ios' + 'yarn test:harness:ios' work without naming a device. Drop the now- redundant device detection from the TSan script.

Booted-sim auto-detection is ambiguous with more than one booted device (it and 'simctl install booted' can disagree). Collect all booted sims and warn which one is used, pointing at DEVICE_MODEL / IOS_VERSION to pin it.

The 3-step TSan flow (build:ios --extra-params, simctl install booted, test:harness:ios) is simple enough and the harness now auto-targets the booted simulator, so the wrapper script just duplicated it (via a different build path).

…rgon

Copilot

Pull request overview

This PR addresses intermittent crashes related to Rive data-binding thread-safety by enforcing single-threaded access patterns across iOS and Android, and adds harness-based regression probes for issue #297.

Changes:

iOS: serialize Rive data-binding/property access onto the main thread (MainThread.run, Promise.onMain) across ViewModel/property wrappers and legacy sync RiveView input/text-run APIs.
Android: make property listener dispatch safe under concurrent add/remove by switching to ConcurrentHashMap and iterating a snapshot.
Tooling/tests: add harness tests + repro screen for issue #297, improve harness simulator selection/TSan options, and bump react-native-nitro-modules to 0.35.9.

Reviewed changes

Copilot reviewed 24 out of 27 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
yarn.lock	Updates lockfile for `react-native-nitro-modules@0.35.9`.
package.json	Bumps `react-native-nitro-modules` dependency + peer range.
ios/Utils.swift	Adds `MainThread.run` and `Promise.onMain` helpers for main-thread serialization.
ios/HybridViewModelTriggerProperty.swift	Routes trigger calls through `MainThread.run`.
ios/HybridViewModelStringProperty.swift	Main-thread wraps sync access; async uses `Promise.onMain`.
ios/HybridViewModelNumberProperty.swift	Main-thread wraps sync access; async uses `Promise.onMain`.
ios/HybridViewModelListProperty.swift	Main-thread wraps list access/mutation; async uses `Promise.onMain`.
ios/HybridViewModelInstance.swift	Main-thread wraps instance/property lookups + replace operations.
ios/HybridViewModelImageProperty.swift	Main-thread wraps image value setting.
ios/HybridViewModelEnumProperty.swift	Main-thread wraps sync access; async uses `Promise.onMain`.
ios/HybridViewModelColorProperty.swift	Main-thread wraps sync access; async uses `Promise.onMain`.
ios/HybridViewModelBooleanProperty.swift	Main-thread wraps sync access; async uses `Promise.onMain`.
ios/HybridViewModelArtboardProperty.swift	Main-thread wraps artboard binding.
ios/HybridViewModel.swift	Main-thread wraps ViewModel operations; async uses `Promise.onMain`.
ios/HybridRiveView.swift	Main-thread wraps legacy sync input/text-run APIs.
ios/BaseHybridViewModelProperty.swift	Main-thread wraps listener add/remove operations.
expo55-example/package.json	Bumps example’s `react-native-nitro-modules` to `0.35.9`.
expo-example/package.json	Bumps example’s `react-native-nitro-modules` to `0.35.9`.
example/src/reproducers/Issue297ThreadRace.tsx	Adds manual in-app stress reproducer for #297.
example/src/tests/issue297.hooks.test.tsx	Adds harness test reproducing the race via public hooks.
example/src/tests/issue297.databinding.test.ts	Adds harness test probe for concurrent data-binding access.
example/rn-harness.config.mjs	Improves simulator targeting and configures TSan env options.
example/package.json	Bumps example’s `react-native-nitro-modules` to `0.35.9`.
example/ios/Podfile.lock	Updates iOS pods (NitroModules/RNRive/etc.) reflecting dependency bump.
eslint.config.mjs	Ignores harness cache directory.
android/src/main/java/com/margelo/nitro/rive/BaseHybridViewModelPropertyImpl.kt	Uses `ConcurrentHashMap` + snapshot iteration to avoid `ConcurrentModificationException`.
.gitignore	Ignores `.harness/` build cache.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  func swap(index1: Double, index2: Double) throws -> Bool {
-    let idx1 = UInt32(index1)
-    let idx2 = UInt32(index2)
-    guard idx1 < property.count && idx2 < property.count else {
-      return false
+    MainThread.run {
+      let idx1 = UInt32(index1)
+      let idx2 = UInt32(index2)
+      guard idx1 < property.count && idx2 < property.count else {


  func getInstanceAt(index: Double) throws -> (any HybridViewModelInstanceSpec)? {
-    guard let instance = property.instance(at: Int32(index)) else { return nil }
-    return HybridViewModelInstance(viewModelInstance: instance)
+    MainThread.run { () -> (any HybridViewModelInstanceSpec)? in
+      guard let instance = property.instance(at: Int32(index)) else { return nil }
+      return HybridViewModelInstance(viewModelInstance: instance)
+    }


  func addInstanceAt(instance: any HybridViewModelInstanceSpec, index: Double) throws -> Bool {
    let viewModelInstance = try requireViewModelInstance(instance)
-    return property.insert(viewModelInstance, at: Int32(index))
+    return MainThread.run { property.insert(viewModelInstance, at: Int32(index)) }
  }


  func removeInstanceAt(index: Double) throws {
-    property.remove(at: Int32(index))
+    MainThread.run { property.remove(at: Int32(index)) }
  }


  func createInstanceByIndex(index: Double) throws -> (any HybridViewModelInstanceSpec)? {
-    guard index >= 0 else { return nil }
-    guard let viewModel = viewModel,
-          let vmi = viewModel.createInstance(fromIndex: UInt(index)) else { return nil }
-    return HybridViewModelInstance(viewModelInstance: vmi)
+    MainThread.run { () -> (any HybridViewModelInstanceSpec)? in
+      guard index >= 0 else { return nil }
+      guard let viewModel = viewModel,
+            let vmi = viewModel.createInstance(fromIndex: UInt(index)) else { return nil }
+      return HybridViewModelInstance(viewModelInstance: vmi)


  func setNumberInputValue(name: String, value: Double, path: String?) throws {
-    try getRiveView().setNumberInputValue(name: name, value: Float(value), path: path)
+    try MainThread.run {
+      try getRiveView().setNumberInputValue(name: name, value: Float(value), path: path)
+    }
  }


mfazekas changed the title ~~fix(ios): make data binding thread-safe (#297)~~ fix: make Rive data binding thread-safe on iOS & Android (#297) Jun 19, 2026

mfazekas added 17 commits June 19, 2026 09:29

docs(test): clarify the JS-thread vs cooperative-pool overlap is the …

8a0698e

…race

chore: trim non-essential code comments; add CLAUDE.md comment guidance

319bfc6

chore: satisfy eslint (prettier formatting, ignore .harness, disable …

e78e926

…no-deprecated in repro)

fix(test): drop unsupported timeout arg from harness it() (TS2554)

bfbf5d6

chore: add gitignored docs/local for local-only notes

f60f2ec

chore: drop repo CLAUDE.md (comment guideline lives in global config)

53d56bd

chore: drop repo docs/local; local notes go in globally-ignored .local/

c83396e

chore(example): warn when multiple simulators are booted

a93f5e6

Booted-sim auto-detection is ambiguous with more than one booted device (it and 'simctl install booted' can disagree). Collect all booted sims and warn which one is used, pointing at DEVICE_MODEL / IOS_VERSION to pin it.

mfazekas force-pushed the fix/issue-297-databinding-thread-safety branch from 92cbb5d to 69526cd Compare June 19, 2026 07:29

mfazekas added 3 commits June 19, 2026 10:22

docs(ios): reword Promise.onMain comment — drop 'already-resolved' ja…

8bd52e6

…rgon

chore: bump react-native-nitro-modules to 0.35.9

b6af540

fix(ios): make Promise.onMain non-blocking (#297)

26fdb21

mfazekas marked this pull request as ready for review June 22, 2026 14:45

mfazekas requested a review from HayesGordon June 22, 2026 14:52

HayesGordon requested a review from Copilot June 23, 2026 09:09

Copilot started reviewing on behalf of HayesGordon June 23, 2026 09:10 View session

Copilot AI reviewed Jun 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: make Rive data binding thread-safe on iOS & Android (#297)#298

fix: make Rive data binding thread-safe on iOS & Android (#297)#298
mfazekas wants to merge 20 commits into
mainfrom
fix/issue-297-databinding-thread-safety

mfazekas commented Jun 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mfazekas commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The fix

Verification (Thread Sanitizer)

Tests

Running the harness tests under Thread Sanitizer (iOS)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mfazekas commented Jun 18, 2026 •

edited

Loading