fix(android): prevent TurboModule event emitter crash before setEventEmitterCallback (#428)#441
fix(android): prevent TurboModule event emitter crash before setEventEmitterCallback (#428)#441AmitKulkarni23 wants to merge 1 commit into
Conversation
|
@AmitKulkarni23 thanks so much for taking the time to make this PR and include such a thorough description. Generally, I think this makes sense and seems pretty safe for us to add. I am curious to better understand how you came across this, if only for my own comprehension. I am able to build the SDK on the new architecture for android using a typical npm download/build approach, which is why I haven't seen it in a typical development flow. |
|
Thanks @alanjcharles14. You are right that this doesn't surface via a typical npm install + build. It's a race condition (timing issue). I was building a proof-of-concept around HorizonDB and exploring Radar's public repos as part of that. Saw issue #428 was open, noticed the iOS fix had already landed in #433 using a jsEventEmitterReady guard, and the Android path seemed similar and a straightforward fix. The hard part was reproducing this - Claude helped me a lot here. |
The Issue
On React Native 0.79+ with
newArchEnabled=true(TurboModules / New Architecture), the app crashes on Android during initialization. The Radar SDK's native receiver callbacks (onClientLocationUpdated,onEventsReceived, etc.) can fire before the TurboModule framework has calledsetEventEmitterCallback, leavingmEventEmitterCallbackasnull. Calling anyemitXxxmethod at that point crashes the process.Affected: Android + New Architecture only. iOS was fixed in #433. Old Architecture unaffected. This PR addresses #428.
Steps to Reproduce
Setup
android/gradle.propertieshas:android/app/src/main/AndroidManifest.xml:android/app/build.gradle(required on emulator — the Radar SDK declares this as a transitive dep but it must be resolvable at runtime):implementation("com.google.android.gms:play-services-location:21.3.0")Workarounds Applied During Reproduction
Two bundler issues required patching
node_modules/react-native-radar/dist/directly for the test app to load:a)
dist/index.js— conditional UI requires caused bundler errors:b) If
npm run build-allwas not run beforenpm pack,dist/ui/map.jsxwould be missing. Fix: render an empty view indist/ui/map.jsx. This is avoided by always runningbuild-allbefore packing.Triggering the Crash
In
App.tsx, call initialize and trackOnce on mount:To make the crash deterministic without waiting for a real GPS fix, an
initblock was injected intoRadarModule.ktto emit an event during construction — beforesetEventEmitterCallbackis called by the framework:Build & Run
Crash Log
Captured on: Pixel 8 emulator, API 36.1 (Google Play ARM64), React Native 0.81,
newArchEnabled=trueRoot cause:
mEventEmitterCallback(set by the TurboModule framework viasetEventEmitterCallback) isnullwhen a receiver callback fires early. AnyemitXxxcall at this point invokesCxxCallbackImpl.invoke()on a null reference, aborting the runtime.The Fix
File:
android/src/newarch/java/com/radar/RadarModule.ktAdd a
@Volatileflag that is set totrueonly aftersetEventEmitterCallbackis called by the framework. Guard everyemitXxxcall site with an early return on this flag.@Volatileprovides the Java memory model guarantee needed here: once the framework thread writestrue, any background thread (GPS callbacks, SDK receivers) is guaranteed to see the updated value and seesmEventEmitterCallbackas fully initialized.Applied to all receiver callbacks:
This matches the approach used for iOS in #433 (
jsEventEmitterReadybool +setEventEmitterCallback:override inside#ifdef RCT_NEW_ARCH_ENABLED).Verification
After applying the fix, rebuilt and reinstalled the APK on the same emulator with the same test app (including the injected
initblock that previously triggered the crash).No crash entries after the fix was deployed. All pre-fix crash lines confirm the before/after boundary:
FATALatRadarModule.<init>(RadarModule.kt:48) — consistent across multiple runs at 18:48, 18:55, 19:00FATAL, noRadarModulecrash lines in logcatNote to Reviewers/Maintainers
First-time contributor here. I used Claude (Anthropic's AI assistant) to help root-cause but I’ve spent significant time manually verifying the mechanics of the fix. I’ve ensured that the @volatile implementation correctly addresses the TurboModule init sequence (specifically handling the different failure modes on Android). I've also completed a full reproduction and manual testing pass on an emulator. Happy to answer questions or make changes based on feedback.