Skip to content

Commit 890e1da

Browse files
Fix Fabric out-of-order event delivery on Android
- Root cause: in FabricUIManager.receiveEvent, a direct dispatch to a newly-ready EventEmitterWrapper could overtake an enqueue lambda for the same tag that was still pending on the main looper, breaking the FIFO contract observed by JS event handlers. - Fix: track in-flight enqueues in SurfaceMountingManager.ViewState via a pendingEventOps AtomicInteger (incremented before posting the lambda, decremented in finally). Expose hasPendingEvents(reactTag) and surface it through MountingManager. In receiveEvent, when the counter is non-zero and the call is not on the synchronous fast path, route through enqueuePendingEvent so ordering is preserved. Synchronous dispatches keep their existing fast path unchanged. - Tests: SurfaceMountingManagerEventOrderingTest (Robolectric, paused looper) covers 6 cases, including an end-to-end FIFO check that enqueues two events while the lambda is in-flight and uses Mockito InOrder against a mocked EventEmitterWrapper to assert delivery order after draining the looper. Fixes #54636
1 parent 0c153e2 commit 890e1da

4 files changed

Lines changed: 245 additions & 8 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1227,6 +1227,21 @@ public void receiveEvent(
12271227
return;
12281228
}
12291229

1230+
// If a previous event for this tag was queued for the UI thread but has not yet been
1231+
// dispatched, route this one through the same path so that JS observes events in receive
1232+
// order. Synchronous dispatches keep their fast path.
1233+
if (!experimentalIsSynchronous && mMountingManager.hasPendingEvents(surfaceId, reactTag)) {
1234+
mMountingManager.enqueuePendingEvent(
1235+
surfaceId,
1236+
reactTag,
1237+
eventName,
1238+
canCoalesceEvent,
1239+
params,
1240+
eventCategory,
1241+
eventTimestamp);
1242+
return;
1243+
}
1244+
12301245
if (experimentalIsSynchronous) {
12311246
UiThreadUtil.assertOnUiThread();
12321247
// add() returns true only if there are no equivalent events already in the set

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,11 @@ internal class MountingManager(
356356
)
357357
}
358358

359+
@AnyThread
360+
@ThreadConfined(ThreadConfined.ANY)
361+
fun hasPendingEvents(surfaceId: Int, reactTag: Int): Boolean =
362+
getSurfaceMountingManager(surfaceId, reactTag)?.hasPendingEvents(reactTag) ?: false
363+
359364
private fun getSurfaceMountingManager(surfaceId: Int, reactTag: Int): SurfaceMountingManager? =
360365
if (surfaceId == ViewUtil.NO_SURFACE_ID) getSurfaceManagerForView(reactTag)
361366
else getSurfaceManager(surfaceId)

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

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import java.util.ArrayDeque
5656
import java.util.LinkedList
5757
import java.util.Queue
5858
import java.util.concurrent.ConcurrentHashMap
59+
import java.util.concurrent.atomic.AtomicInteger
5960
import kotlin.concurrent.Volatile
6061

6162
/** Returns true if the collection contains [key]. */
@@ -1151,19 +1152,35 @@ internal constructor(
11511152

11521153
val viewEvent =
11531154
PendingViewEvent(eventName, params, eventCategory, canCoalesceEvent, eventTimestamp)
1155+
// Mark that an enqueue is in flight before posting; readers on other threads must observe this
1156+
// before they decide to dispatch directly, so events stay in receive order.
1157+
viewState.pendingEventOps.incrementAndGet()
11541158
UiThreadUtil.runOnUiThread {
1155-
val eventEmitter = viewState.eventEmitter
1156-
if (eventEmitter != null) {
1157-
viewEvent.dispatch(eventEmitter)
1158-
} else {
1159-
val queue =
1160-
viewState.pendingEventQueue
1161-
?: LinkedList<PendingViewEvent>().also { viewState.pendingEventQueue = it }
1162-
queue.add(viewEvent)
1159+
try {
1160+
val eventEmitter = viewState.eventEmitter
1161+
if (eventEmitter != null) {
1162+
viewEvent.dispatch(eventEmitter)
1163+
} else {
1164+
val queue =
1165+
viewState.pendingEventQueue
1166+
?: LinkedList<PendingViewEvent>().also { viewState.pendingEventQueue = it }
1167+
queue.add(viewEvent)
1168+
}
1169+
} finally {
1170+
viewState.pendingEventOps.decrementAndGet()
11631171
}
11641172
}
11651173
}
11661174

1175+
/**
1176+
* Returns true if an enqueuePendingEvent call for [reactTag] has been posted to the UI thread but
1177+
* has not yet executed. Callers should route subsequent events through [enqueuePendingEvent] in
1178+
* that case to preserve receive order.
1179+
*/
1180+
@AnyThread
1181+
internal fun hasPendingEvents(reactTag: Int): Boolean =
1182+
(tagToViewState[reactTag]?.pendingEventOps?.get() ?: 0) > 0
1183+
11671184
public fun markActiveTouchForTag(reactTag: Int): Unit {
11681185
viewsWithActiveTouches.add(reactTag)
11691186
}
@@ -1192,6 +1209,10 @@ internal constructor(
11921209

11931210
@ThreadConfined(ThreadConfined.UI) var pendingEventQueue: Queue<PendingViewEvent>? = null
11941211

1212+
// Tracks enqueuePendingEvent operations posted to the UI thread but not yet executed. Read from
1213+
// any thread to detect in-flight events that must not be bypassed by a direct dispatch.
1214+
val pendingEventOps: AtomicInteger = AtomicInteger(0)
1215+
11951216
override fun toString(): String {
11961217
val isLayoutOnly = viewManager == null
11971218
return "ViewState [$reactTag] - isRoot: $isRoot - props: $currentProps - viewManager: $viewManager - isLayoutOnly: $isLayoutOnly"
Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
@file:Suppress("DEPRECATION")
9+
10+
package com.facebook.react.fabric
11+
12+
import android.os.Looper
13+
import com.facebook.react.ReactRootView
14+
import com.facebook.react.bridge.JavaOnlyMap
15+
import com.facebook.react.bridge.ReactTestHelper
16+
import com.facebook.react.fabric.events.EventEmitterWrapper
17+
import com.facebook.react.fabric.mounting.MountingManager
18+
import com.facebook.react.fabric.mounting.MountingManager.MountItemExecutor
19+
import com.facebook.react.fabric.mounting.SurfaceMountingManager
20+
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlagsForTests
21+
import com.facebook.react.uimanager.ThemedReactContext
22+
import com.facebook.react.uimanager.ViewManager
23+
import com.facebook.react.uimanager.ViewManagerRegistry
24+
import com.facebook.react.uimanager.events.EventCategoryDef
25+
import com.facebook.react.views.view.ReactViewManager
26+
import com.facebook.testutils.shadows.ShadowSoLoader
27+
import org.assertj.core.api.Assertions.assertThat
28+
import org.junit.Before
29+
import org.junit.Test
30+
import org.junit.runner.RunWith
31+
import org.mockito.Mockito.inOrder
32+
import org.mockito.kotlin.mock
33+
import org.robolectric.RobolectricTestRunner
34+
import org.robolectric.Shadows.shadowOf
35+
import org.robolectric.annotation.Config
36+
import org.robolectric.annotation.LooperMode
37+
38+
/**
39+
* Regression tests for issue #54636: events emitted before [SurfaceMountingManager.updateEventEmitter]
40+
* has run must reach JS in receive order, even if a later event happens to find the emitter ready.
41+
*
42+
* The fix relies on [SurfaceMountingManager.hasPendingEvents] reflecting that an enqueue lambda
43+
* has been posted to the UI thread but not yet executed, so callers can route subsequent events
44+
* through the same queue path instead of dispatching them directly.
45+
*/
46+
@RunWith(RobolectricTestRunner::class)
47+
@LooperMode(LooperMode.Mode.PAUSED)
48+
@Config(shadows = [ShadowSoLoader::class])
49+
class SurfaceMountingManagerEventOrderingTest {
50+
private lateinit var mountingManager: MountingManager
51+
private lateinit var themedReactContext: ThemedReactContext
52+
private val surfaceId = 1
53+
private val reactTag = 42
54+
55+
@Before
56+
fun setUp() {
57+
ReactNativeFeatureFlagsForTests.setUp()
58+
val reactContext = ReactTestHelper.createCatalystContextForTest()
59+
themedReactContext = ThemedReactContext(reactContext, reactContext, null, -1)
60+
mountingManager =
61+
MountingManager(
62+
ViewManagerRegistry(listOf<ViewManager<*, *>>(ReactViewManager())),
63+
MountItemExecutor {},
64+
)
65+
}
66+
67+
private fun startSurfaceWithView(): SurfaceMountingManager {
68+
val rootView = ReactRootView(themedReactContext)
69+
mountingManager.startSurface(surfaceId, themedReactContext, rootView)
70+
val smm = mountingManager.getSurfaceManagerEnforced(surfaceId, "test")
71+
smm.preallocateView("RCTView", reactTag, JavaOnlyMap.of(), null, true)
72+
return smm
73+
}
74+
75+
/**
76+
* After enqueuePendingEvent posts its UI-thread lambda, hasPendingEvents must return true so
77+
* that any concurrent direct-dispatch path knows to also queue rather than overtake.
78+
*/
79+
@Test
80+
fun hasPendingEvents_isTrue_whileEnqueueLambdaIsInFlight() {
81+
val smm = startSurfaceWithView()
82+
83+
assertThat(smm.hasPendingEvents(reactTag)).isFalse()
84+
85+
smm.enqueuePendingEvent(
86+
reactTag, "topChange", false, null, EventCategoryDef.UNSPECIFIED, 0L)
87+
88+
assertThat(smm.hasPendingEvents(reactTag)).isTrue()
89+
90+
shadowOf(Looper.getMainLooper()).idle()
91+
92+
assertThat(smm.hasPendingEvents(reactTag)).isFalse()
93+
}
94+
95+
/**
96+
* MountingManager.hasPendingEvents must mirror the SurfaceMountingManager state so callers above
97+
* the surface layer (e.g. FabricUIManager.receiveEvent) can consult it.
98+
*/
99+
@Test
100+
fun mountingManager_hasPendingEvents_mirrorsSurfaceMountingManager() {
101+
startSurfaceWithView()
102+
103+
assertThat(mountingManager.hasPendingEvents(surfaceId, reactTag)).isFalse()
104+
105+
mountingManager.enqueuePendingEvent(
106+
surfaceId, reactTag, "topChange", false, null, EventCategoryDef.UNSPECIFIED, 0L)
107+
108+
assertThat(mountingManager.hasPendingEvents(surfaceId, reactTag)).isTrue()
109+
110+
shadowOf(Looper.getMainLooper()).idle()
111+
112+
assertThat(mountingManager.hasPendingEvents(surfaceId, reactTag)).isFalse()
113+
}
114+
115+
/**
116+
* The counter must match the number of in-flight enqueue lambdas, so that interleaved enqueue
117+
* + direct-route-to-enqueue calls are all accounted for and only fall back to false once the UI
118+
* thread has fully drained them.
119+
*/
120+
@Test
121+
fun hasPendingEvents_remainsTrue_acrossMultipleEnqueues() {
122+
val smm = startSurfaceWithView()
123+
124+
smm.enqueuePendingEvent(
125+
reactTag, "topChange", false, null, EventCategoryDef.UNSPECIFIED, 0L)
126+
smm.enqueuePendingEvent(
127+
reactTag, "topChange", false, null, EventCategoryDef.UNSPECIFIED, 1L)
128+
smm.enqueuePendingEvent(
129+
reactTag, "topChange", false, null, EventCategoryDef.UNSPECIFIED, 2L)
130+
131+
assertThat(smm.hasPendingEvents(reactTag)).isTrue()
132+
133+
shadowOf(Looper.getMainLooper()).idle()
134+
135+
assertThat(smm.hasPendingEvents(reactTag)).isFalse()
136+
}
137+
138+
/** Tags with no enqueue activity must not report pending events. */
139+
@Test
140+
fun hasPendingEvents_isFalse_forUnrelatedTag() {
141+
val smm = startSurfaceWithView()
142+
val otherTag = reactTag + 1
143+
smm.preallocateView("RCTView", otherTag, JavaOnlyMap.of(), null, true)
144+
145+
smm.enqueuePendingEvent(
146+
reactTag, "topChange", false, null, EventCategoryDef.UNSPECIFIED, 0L)
147+
148+
assertThat(smm.hasPendingEvents(reactTag)).isTrue()
149+
assertThat(smm.hasPendingEvents(otherTag)).isFalse()
150+
151+
shadowOf(Looper.getMainLooper()).idle()
152+
}
153+
154+
/** Calling enqueuePendingEvent for a tag without view state must not falsely flag it. */
155+
@Test
156+
fun hasPendingEvents_isFalse_forUnknownTag() {
157+
startSurfaceWithView()
158+
val unknownTag = reactTag + 99
159+
160+
mountingManager.enqueuePendingEvent(
161+
surfaceId, unknownTag, "topChange", false, null, EventCategoryDef.UNSPECIFIED, 0L)
162+
163+
assertThat(mountingManager.hasPendingEvents(surfaceId, unknownTag)).isFalse()
164+
}
165+
166+
/**
167+
* End-to-end ordering invariant: when a caller observes [hasPendingEvents] is true and routes a
168+
* subsequent event through [enqueuePendingEvent] (instead of dispatching directly), the emitter
169+
* must observe the events in receive order. This is the contract the [FabricUIManager.receiveEvent]
170+
* fix relies on to prevent #54636.
171+
*/
172+
@Test
173+
fun enqueuePendingEvent_dispatchesInFifoOrder_whenLambdaIsInFlight() {
174+
val smm = startSurfaceWithView()
175+
val emitter: EventEmitterWrapper = mock()
176+
177+
smm.updateEventEmitter(reactTag, emitter)
178+
179+
shadowOf(Looper.getMainLooper()).pause()
180+
181+
smm.enqueuePendingEvent(reactTag, "first", false, null, EventCategoryDef.UNSPECIFIED, 1L)
182+
assertThat(smm.hasPendingEvents(reactTag)).isTrue()
183+
184+
// Caller (e.g. FabricUIManager.receiveEvent) sees a previous enqueue is still in-flight and
185+
// routes through enqueuePendingEvent rather than dispatching directly via the emitter.
186+
smm.enqueuePendingEvent(reactTag, "second", false, null, EventCategoryDef.UNSPECIFIED, 2L)
187+
188+
shadowOf(Looper.getMainLooper()).idle()
189+
190+
val ordered = inOrder(emitter)
191+
ordered.verify(emitter).dispatch("first", null, EventCategoryDef.UNSPECIFIED, 1L)
192+
ordered.verify(emitter).dispatch("second", null, EventCategoryDef.UNSPECIFIED, 2L)
193+
194+
assertThat(smm.hasPendingEvents(reactTag)).isFalse()
195+
}
196+
}

0 commit comments

Comments
 (0)