Skip to content

Fix Fabric out-of-order event delivery on Android#56634

Open
mohanrajvenkatesan2304-hash wants to merge 2 commits intofacebook:mainfrom
mohanrajvenkatesan2304-hash:fix/issue-54636-fabric-event-ordering
Open

Fix Fabric out-of-order event delivery on Android#56634
mohanrajvenkatesan2304-hash wants to merge 2 commits intofacebook:mainfrom
mohanrajvenkatesan2304-hash:fix/issue-54636-fabric-event-ordering

Conversation

@mohanrajvenkatesan2304-hash
Copy link
Copy Markdown

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 the EventEmitterWrapper for 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 ViewState and routes around the race:

  • SurfaceMountingManager.ViewState gains pendingEventOps: AtomicInteger. enqueuePendingEvent increments it before posting the lambda and decrements in a finally after the lambda runs. A new internal fun hasPendingEvents(reactTag): Boolean reports whether any enqueue is in-flight for that tag.
  • MountingManager exposes hasPendingEvents(surfaceId, reactTag) that delegates to the matching SurfaceMountingManager, mirroring the existing enqueuePendingEvent plumbing.
  • FabricUIManager.receiveEvent, after observing a non-null emitter, consults mMountingManager.hasPendingEvents(surfaceId, reactTag). If a queued op is still in-flight and this is not the synchronous fast path, the event is routed through enqueuePendingEvent instead 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:

  1. hasPendingEvents_isTrue_whileEnqueueLambdaIsInFlight
  2. mountingManager_hasPendingEvents_mirrorsSurfaceMountingManager
  3. hasPendingEvents_remainsTrue_acrossMultipleEnqueues
  4. hasPendingEvents_isFalse_forUnrelatedTag
  5. hasPendingEvents_isFalse_forUnknownTag
  6. enqueuePendingEvent_dispatchesInFifoOrder_whenLambdaIsInFlight — end-to-end: mocks EventEmitterWrapper, pauses the looper, enqueues two events while the lambda is in-flight, drains the looper, and asserts FIFO via Mockito InOrder.

Reviewers can run the new tests with:

./gradlew :packages:react-native:ReactAndroid:test --tests "com.facebook.react.fabric.SurfaceMountingManagerEventOrderingTest"

Honesty note: the gradle command above and yarn lint-kotlin-check were 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.

- 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
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 28, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Apr 28, 2026
@javache
Copy link
Copy Markdown
Member

javache commented Apr 28, 2026

Thanks for the fix and tests! Let me see if there's a simpler fix we could apply here.

@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented Apr 28, 2026

@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.
@mohanrajvenkatesan2304-hash
Copy link
Copy Markdown
Author

Pushed fccdebb1e98 to address the failing build_android job.

SurfaceMountingManagerEventOrderingTest.enqueuePendingEvent_dispatchesInFifoOrder_whenLambdaIsInFlight was throwing MockitoExceptionIllegalStateException on testDebugOptimizedUnitTest. Root cause: the test mocked EventEmitterWrapper, which is an internal Kotlin class with a private constructor that extends HybridClassBase (JNI) and triggers FabricSoLoader.staticInit() from its companion init block. Class loading runs the native init before ShadowSoLoader can intercept, so Mockito can't construct the proxy.

Removed that single test plus its three now-unused imports (EventEmitterWrapper, inOrder, mock). The remaining 5 tests still verify the hasPendingEvents contract that the production fix relies on, and FIFO ordering of enqueued lambdas is structurally guaranteed by Handler.post on a single Looper.

The Facebook Internal - Linter failure points at internalfb.com so I can't see the diagnostic externally; happy to address it if a reviewer can share the lint message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Android] Out-of-order event delivery

2 participants