fix(Reanimated): Avoid ObjectAlreadyConsumedException when handling events on Android#9686
fix(Reanimated): Avoid ObjectAlreadyConsumedException when handling events on Android#9686tomekzaw wants to merge 3 commits into
Conversation
…vents on Android handleEvent consumed the event payload map, which is owned by the RN event and reused for the dispatch to JS. Skip events with no subscribed handler and consume a copy so the original is left intact. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes an Android crash in Reanimated’s native event handling where consuming the incoming WritableNativeMap payload (via NativeMap::consume()) can corrupt downstream React Native event dispatch and trigger ObjectAlreadyConsumedException.
Changes:
- Adds an early subscription check in
NativeProxy::handleEventto skip payload conversion work when no handler is waiting. - Copies the event payload map before consuming it, preserving the original for React Native’s own dispatch.
- Reuses a single
eventNameStrto avoid repeated string conversions.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| auto eventNameStr = eventName->toString(); | ||
| if (!reanimatedModuleProxy_->isAnyHandlerWaitingForEvent(eventNameStr, emitterReactTag)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Good catch, fixed in 485c982. isAnyHandlerWaitingForEvent now also checks eventMappingsWithoutTag, matching processEvent. This also closes the same pre-existing gap where the off-thread path in NodesManager and handleRawEvent ignored tagless handlers.
| static const auto copyMethod = | ||
| react::WritableMap::javaClassStatic()->getMethod<react::WritableMap::javaobject()>("copy"); | ||
| auto nativeMap = jni::static_ref_cast<react::WritableNativeMap::javaobject>( | ||
| jni::static_ref_cast<jobject>(copyMethod(event))); | ||
| auto eventPayload = nativeMap->cthis()->consume(); |
There was a problem hiding this comment.
event is already guarded by isInstanceOf(WritableNativeMap) just above, and WritableNativeMap.copy() returns a WritableNativeMap, so the cast cannot fail here. Leaving it as is rather than adding a branch that is unreachable.
isAnyHandlerWaitingForEvent only checked handlers registered with an emitterReactTag, while processEvent also dispatches to handlers registered without one (emitterReactTag = -1). The handler gate in handleEvent would therefore drop events that only have a tagless handler. Match processEvent by also checking eventMappingsWithoutTag. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| const auto withoutTagIt = eventMappingsWithoutTag.find(eventName); | ||
| if (withoutTagIt != eventMappingsWithoutTag.end() && !withoutTagIt->second.empty()) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Are we sure this is fine? We might hit performOperations more often now.
…p in `handleEvent`" (software-mansion#9687) Reverts software-mansion#9581 software-mansion#9581 made `handleEvent` consume the incoming event payload map via `NativeMap::consume()`, which moves out its `folly::dynamic` and marks the map consumed. That map is owned by the React Native event and is also dispatched to JS, and some events return a cached instance from `getEventData()`, so consuming it in Reanimated corrupts the downstream dispatch and throws `ObjectAlreadyConsumedException: Map already consumed` on Android. Reverting to be safe for the 4.5.0 release; a fixed version that keeps the optimization is in software-mansion#9686. Confirmed crashes: MapLibre `MapChangeEvent`: ``` com.facebook.react.bridge.ObjectAlreadyConsumedException: Map already consumed at com.facebook.react.fabric.events.EventEmitterWrapper.dispatchUniqueEvent(Native Method) at com.facebook.react.fabric.events.EventEmitterWrapper.dispatchUnique(EventEmitterWrapper.kt:71) at com.facebook.react.fabric.FabricUIManager.receiveEvent(FabricUIManager.java:1149) at com.facebook.react.fabric.events.FabricEventEmitter.receiveEvent(FabricEventEmitter.kt:28) at com.facebook.react.uimanager.events.Event.dispatchModern(Event.kt:192) at com.facebook.react.uimanager.events.FabricEventDispatcher.dispatchEvent(FabricEventDispatcher.kt:51) at org.maplibre.reactnative.components.mapview.MLRNMapView.handleMapChangedEvent(MLRNMapView.kt:1513) at org.maplibre.reactnative.components.mapview.MLRNMapView.onWillStartRenderingMap(MLRNMapView.kt:731) ``` Expo view events (`KEventEmitterWrapper.UIEvent`, for example expo-image): ``` com.facebook.react.bridge.ObjectAlreadyConsumedException: Map already consumed at com.facebook.react.fabric.events.EventEmitterWrapper.dispatchEvent(Native Method) at com.facebook.react.fabric.events.EventEmitterWrapper.dispatch(EventEmitterWrapper.kt:47) at com.facebook.react.fabric.FabricUIManager.receiveEvent(FabricUIManager.java:1151) at com.facebook.react.fabric.events.FabricEventEmitter.receiveEvent(FabricEventEmitter.kt:28) at com.facebook.react.uimanager.events.Event.dispatchModern(Event.kt:192) at com.facebook.react.uimanager.events.FabricEventDispatcher.dispatchEvent(FabricEventDispatcher.kt:51) at expo.modules.kotlin.events.KEventEmitterWrapper.emit(KModuleEventEmitterWrapper.kt:109) at expo.modules.kotlin.viewevent.ViewEvent.invoke(ViewEvent.kt:47) at expo.modules.image.events.GlideRequestListener$onResourceReady$1.invokeSuspend(GlideRequestListener.kt:61) ```
Summary
On Android,
handleEventconsumes the event payload map viaNativeMap::consume(), which was introduced in #9581.consume()moves out the underlyingfolly::dynamicand marks the map consumed. That map is owned by the React Native event and is reused: it is also dispatched to JS, and some events return a cached instance fromgetEventData(). Consuming it in Reanimated corrupts the downstream dispatch and throwsObjectAlreadyConsumedException: Map already consumed.Reanimated observes events as an
EventDispatcherListener, so its listener runs before the real dispatch to JS. Events whosegetEventData()returns a fresh map on every call (such as React Native'sScrollEvent) are unaffected, which is why this was not caught earlier. Events that return a cached map crash. Two confirmed in the wild:MapChangeEvent, which returns a storedeventData:KEventEmitterWrapper.UIEvent, which returns a storedeventBodyand is the base class for every expo-modules-core view event (for example expo-image):This PR changes
handleEventto:handleEventpreviously consumed and converted every event's payload before checking subscription (the check lives inprocessEvent), so unrelated events were destroyed as collateral. This fixes those crashes and also avoids converting payloads that nothing is listening to.Alternatives considered
jsi::Valuewithout a JSON round-trip inhandleEvent#9581 to the JSON path (toString()followed bycreateFromJsonUtf8). Non-destructive and correct, but loses the conversion speedup from perf: Convert events tojsi::Valuewithout a JSON round-trip inhandleEvent#9581 and reintroduces the dropping of events carrying NaN or INF values ([iOS] Causes folly::toJson Exception when used in conjunction with react-native-ivs #1776).MapChangeEvent.getEventData().copy(). Fixes MapLibre but is per-library whack-a-mole: expo-image still crashed through the same path, and the same applies to any other library that returns a cached map.params.copy()fromEventHandler. General, but copies on every event Reanimated observes, including the scroll and gesture hot path.The chosen approach copies only when a handler is actually subscribed, so the common case (no handler) does no work, and the copy is paid only for events that are being animated.
Test plan
On Android with the new architecture, render a component that dispatches an event whose
getEventData()returns a cached map while Reanimated is installed, for example a MapLibreMapView(onWillStartRenderingMap) or an expo-imageImage(onLoad). Before this change the app crashes with the trace above as soon as the event fires; after it, the event is delivered normally.useAnimatedScrollHandlercontinues to receive scroll events, which use the fresh-map path.🤖 Generated with Claude Code