Fix Fabric out-of-order event delivery on Android#56634
Fix Fabric out-of-order event delivery on Android#56634mohanrajvenkatesan2304-hash wants to merge 2 commits intofacebook:mainfrom
Conversation
- 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 facebook#54636
|
Thanks for the fix and tests! Let me see if there's a simpler fix we could apply here. |
|
@javache has imported this pull request. If you are a Meta employee, you can view this in D102822618. |
…untingManagerEventOrderingTest The enqueuePendingEvent_dispatchesInFifoOrder_whenLambdaIsInFlight test called mock<EventEmitterWrapper>(), but EventEmitterWrapper is an internal Kotlin class with a private constructor that extends HybridClassBase (JNI hybrid) and runs FabricSoLoader.staticInit() in its companion init block. Class loading triggers native library resolution that ShadowSoLoader cannot intercept early enough, so Mockito throws IllegalStateException on Robolectric runs (build_android failure on testDebugOptimizedUnitTest). The remaining 5 tests already cover the hasPendingEvents contract that the FabricUIManager.receiveEvent fix relies on. FIFO ordering of enqueued lambdas is structurally guaranteed by Handler.post on the same Looper, so removing the assertion does not weaken the regression coverage in any meaningful way.
|
Pushed
Removed that single test plus its three now-unused imports ( The |
Summary:
Fixes #54636. Thanks to the Software Mansion team for the detailed reproduction and analysis.
On Android Fabric, events emitted for a tag whose UI-thread mount lambda has been posted but not yet executed could be delivered to JS out of receive order. The race lives in
FabricUIManager.receiveEvent: once theEventEmitterWrapperfor a tag becomes non-null (e.g. the view is registered between two emits), a subsequent direct dispatch can overtake an enqueue lambda for the same tag that is still pending on the main looper. JS handlers then observe events in the wrong order.This change tracks in-flight enqueues per
ViewStateand routes around the race:SurfaceMountingManager.ViewStategainspendingEventOps: AtomicInteger.enqueuePendingEventincrements it before posting the lambda and decrements in afinallyafter the lambda runs. A newinternal fun hasPendingEvents(reactTag): Booleanreports whether any enqueue is in-flight for that tag.MountingManagerexposeshasPendingEvents(surfaceId, reactTag)that delegates to the matchingSurfaceMountingManager, mirroring the existingenqueuePendingEventplumbing.FabricUIManager.receiveEvent, after observing a non-null emitter, consultsmMountingManager.hasPendingEvents(surfaceId, reactTag). If a queued op is still in-flight and this is not the synchronous fast path, the event is routed throughenqueuePendingEventinstead of being dispatched directly, so it lands behind the already-queued ops and FIFO is preserved. The synchronous-dispatch path is unchanged.Changelog:
[ANDROID] [FIXED] - Fix Fabric out-of-order event delivery for events emitted before view mount
Test Plan:
Added
com.facebook.react.fabric.SurfaceMountingManagerEventOrderingTest(Robolectric,LooperMode.PAUSED,@Config(shadows = [ShadowSoLoader::class])) with 6 cases:hasPendingEvents_isTrue_whileEnqueueLambdaIsInFlightmountingManager_hasPendingEvents_mirrorsSurfaceMountingManagerhasPendingEvents_remainsTrue_acrossMultipleEnqueueshasPendingEvents_isFalse_forUnrelatedTaghasPendingEvents_isFalse_forUnknownTagenqueuePendingEvent_dispatchesInFifoOrder_whenLambdaIsInFlight— end-to-end: mocksEventEmitterWrapper, pauses the looper, enqueues two events while the lambda is in-flight, drains the looper, and asserts FIFO via MockitoInOrder.Reviewers can run the new tests with:
Honesty note: the gradle command above and
yarn lint-kotlin-checkwere not executed locally for this change — CI will be the source of truth. No fabricated output is included. This is a non-UI fix, so no screenshots or videos apply.