Skip to content

Commit a3e87c6

Browse files
javachemohanrajvenkatesan23-04
authored andcommitted
Fix Fabric out-of-order event delivery on Android
Summary: Fixes #54636. Thanks to the Software Mansion team for the detailed reproduction and analysis. Based on #56634 On Android Fabric, events emitted for a tag whose `EventEmitterWrapper` has not yet been set (e.g. the view is preallocated but not mounted) are buffered in a per-ViewState queue. When `updateEventEmitter` later sets the emitter, it drains the queue. However, events arriving between the queue-post and the drain could bypass the queue and dispatch directly, delivering events to JS out of receive order. This change replaces the lambda-based `enqueuePendingEvent` (which deferred event buffering to a `runOnUiThread` lambda, leaving events "in limbo" in the Handler queue) with direct queue insertion under a `synchronized(viewState)` lock: - `dispatchEvent` fast path: two volatile reads (`eventEmitter`, `pendingEventQueue`). If the emitter is set and no queue exists, dispatch directly — no lock, no allocation. - `dispatchEvent` slow path (rare, during view init): `synchronized(viewState)` — re-check emitter under lock; if still null, create queue and enqueue the event. If the emitter became available between the volatile read and the lock, a barrier (`synchronized<Unit>(viewState) {}`) waits for any in-progress drain before dispatching. - `updateEventEmitter`: `synchronized(viewState)` — set emitter, drain queue, null the queue reference. The queue stays non-null during the drain, so concurrent fast-path checks correctly fall through to the barrier. Queue nullability (`pendingEventQueue == null`) is the cross-thread signal that replaces the previous `AtomicInteger` counter approach. ## Changelog: [ANDROID][FIXED] - Fix Fabric out-of-order event delivery for events emitted before view mount [ANDROID][BREAKING] - Removed SurfaceMountingManager#enqueuePendingEvent Reviewed By: mdvacca Differential Revision: D102822618 fbshipit-source-id: d683ef50e8c6bcd52cc947b3eda9cb2c1e1296ff Co-authored-by: Mohanraj Venkatesan <mohanraj.venkatesan2304@gmail.com>
1 parent 661ce06 commit a3e87c6

3 files changed

Lines changed: 391 additions & 84 deletions

File tree

packages/react-native/ReactAndroid/api/ReactAndroid.api

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2281,8 +2281,6 @@ public final class com/facebook/react/fabric/mounting/SurfaceMountingManager {
22812281
public final fun applyViewSnapshot (ILandroid/graphics/Bitmap;)V
22822282
public final fun attachRootView (Landroid/view/View;Lcom/facebook/react/uimanager/ThemedReactContext;)V
22832283
public final fun deleteView (I)V
2284-
public final fun enqueuePendingEvent (ILjava/lang/String;ZLcom/facebook/react/bridge/WritableMap;I)V
2285-
public final fun enqueuePendingEvent (ILjava/lang/String;ZLcom/facebook/react/bridge/WritableMap;IJ)V
22862284
public final fun getContext ()Lcom/facebook/react/uimanager/ThemedReactContext;
22872285
public final fun getSurfaceId ()I
22882286
public final fun getView (I)Landroid/view/View;

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt

Lines changed: 38 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ package com.facebook.react.fabric.mounting
99

1010
import android.annotation.SuppressLint
1111
import android.graphics.Bitmap
12-
import android.os.SystemClock
1312
import android.view.View
1413
import android.view.ViewGroup
1514
import android.view.ViewParent
@@ -965,22 +964,19 @@ internal constructor(
965964
}
966965
vs
967966
}
967+
968968
val previousEventEmitterWrapper = viewState.eventEmitter
969-
viewState.eventEmitter = eventEmitter
969+
synchronized(viewState) {
970+
viewState.eventEmitter = eventEmitter
971+
// Invoke pending events queued to the view state
972+
viewState.pendingEventQueue?.forEach { it.dispatch(eventEmitter) }
973+
viewState.pendingEventQueue = null
974+
}
970975

971976
// Immediately destroy native side of wrapper, instead of waiting for Java GC.
972977
if (previousEventEmitterWrapper != eventEmitter && previousEventEmitterWrapper != null) {
973978
previousEventEmitterWrapper.destroy()
974979
}
975-
976-
val pendingEventQueue = viewState.pendingEventQueue
977-
if (pendingEventQueue != null) {
978-
// Invoke pending event queued to the view state
979-
for (viewEvent in pendingEventQueue) {
980-
viewEvent.dispatch(eventEmitter)
981-
}
982-
viewState.pendingEventQueue = null
983-
}
984980
}
985981

986982
@UiThread
@@ -1190,57 +1186,6 @@ internal constructor(
11901186
}
11911187
}
11921188

1193-
@AnyThread
1194-
public fun enqueuePendingEvent(
1195-
reactTag: Int,
1196-
eventName: String,
1197-
canCoalesceEvent: Boolean,
1198-
params: WritableMap?,
1199-
@EventCategoryDef eventCategory: Int,
1200-
): Unit {
1201-
enqueuePendingEvent(
1202-
reactTag,
1203-
eventName,
1204-
canCoalesceEvent,
1205-
params,
1206-
eventCategory,
1207-
SystemClock.uptimeMillis(),
1208-
)
1209-
}
1210-
1211-
@AnyThread
1212-
public fun enqueuePendingEvent(
1213-
reactTag: Int,
1214-
eventName: String,
1215-
canCoalesceEvent: Boolean,
1216-
params: WritableMap?,
1217-
@EventCategoryDef eventCategory: Int,
1218-
eventTimestamp: Long,
1219-
): Unit {
1220-
// When the surface stopped we will reset the view state map. We are not going to enqueue
1221-
// pending events as they are not expected to be dispatched anyways.
1222-
val viewState = registryGet(reactTag)
1223-
1224-
if (viewState == null) {
1225-
// Cannot queue event without view state. Do nothing here.
1226-
return
1227-
}
1228-
1229-
val viewEvent =
1230-
PendingViewEvent(eventName, params, eventCategory, canCoalesceEvent, eventTimestamp)
1231-
UiThreadUtil.runOnUiThread {
1232-
val eventEmitter = viewState.eventEmitter
1233-
if (eventEmitter != null) {
1234-
viewEvent.dispatch(eventEmitter)
1235-
} else {
1236-
val queue =
1237-
viewState.pendingEventQueue
1238-
?: LinkedList<PendingViewEvent>().also { viewState.pendingEventQueue = it }
1239-
queue.add(viewEvent)
1240-
}
1241-
}
1242-
}
1243-
12441189
@AnyThread
12451190
internal fun dispatchEvent(
12461191
reactTag: Int,
@@ -1252,31 +1197,43 @@ internal constructor(
12521197
) {
12531198
val viewState = registryGet(reactTag)
12541199
if (viewState == null) {
1255-
// This can happen if the view has disappeared from the screen (because of async events)
12561200
FLog.i(TAG, "Unable to invoke event: %s for reactTag: %d", eventName, reactTag)
12571201
return
12581202
}
12591203

1260-
val eventEmitter = viewState.eventEmitter
1261-
if (eventEmitter == null) {
1262-
// The view is pre-allocated and created. However, it hasn't been mounted yet. We will have
1263-
// access to the event emitter later when the view is mounted. For now just save the event
1264-
// in the view state and trigger it later.
1265-
enqueuePendingEvent(
1266-
reactTag,
1267-
eventName,
1268-
canCoalesceEvent,
1269-
params,
1270-
eventCategory,
1271-
eventTimestamp,
1272-
)
1273-
return
1204+
var emitter = viewState.eventEmitter
1205+
if (emitter == null) {
1206+
synchronized(viewState) {
1207+
val emitterUnderLock = viewState.eventEmitter
1208+
if (emitterUnderLock != null) {
1209+
emitter = emitterUnderLock
1210+
} else {
1211+
// The view is pre-allocated and created. However, it hasn't been mounted yet. We will
1212+
// have access to the event emitter later when the view is mounted. For now just save the
1213+
// event in the view state and trigger it later.
1214+
val queue =
1215+
viewState.pendingEventQueue
1216+
?: LinkedList<PendingViewEvent>().also { viewState.pendingEventQueue = it }
1217+
queue.add(
1218+
PendingViewEvent(eventName, params, eventCategory, canCoalesceEvent, eventTimestamp)
1219+
)
1220+
return
1221+
}
1222+
}
12741223
}
12751224

1225+
if (viewState.pendingEventQueue != null) {
1226+
// We have an emitter and also a pending event queue - we must lock
1227+
// and wait for the pending event queue to be cleared.
1228+
synchronized<Unit>(viewState) {}
1229+
assert(viewState.pendingEventQueue == null)
1230+
}
1231+
1232+
checkNotNull(emitter)
12761233
if (canCoalesceEvent) {
1277-
eventEmitter.dispatchUnique(eventName, params, eventTimestamp)
1234+
emitter.dispatchUnique(eventName, params, eventTimestamp)
12781235
} else {
1279-
eventEmitter.dispatch(eventName, params, eventCategory, eventTimestamp)
1236+
emitter.dispatch(eventName, params, eventCategory, eventTimestamp)
12801237
}
12811238
}
12821239

@@ -1304,9 +1261,8 @@ internal constructor(
13041261
) {
13051262
var currentProps: ReactStylesDiffMap? = null
13061263
var stateWrapper: StateWrapper? = null
1307-
var eventEmitter: EventEmitterWrapper? = null
1308-
1309-
@ThreadConfined(ThreadConfined.UI) var pendingEventQueue: Queue<PendingViewEvent>? = null
1264+
@Volatile var eventEmitter: EventEmitterWrapper? = null
1265+
@Volatile var pendingEventQueue: Queue<PendingViewEvent>? = null
13101266

13111267
override fun toString(): String {
13121268
val isLayoutOnly = viewManager == null

0 commit comments

Comments
 (0)