React/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2024-11-21 22:10:14 +00:00
Code Issues Actions 10 Packages Projects Releases Wiki Activity

fix race condition in EventBeat (#47548)

Browse Source 
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/47548

changelog: [internal]

This race condition only shows itself with flag `useOptimizedEventBatchingOnAndroid`

# Problem
EventBeat assumes method `induce` will be called repeatedly on every UI tick. This is true for iOS and existing implementation of event beat on Android. The first early exist inside of `induce` method is built with this assumption.

`useOptimizedEventBatchingOnAndroid` on Android changes this. `induce` will only be called after FabricUIManager.onRequestEventBeat is invoked and then it will stop. For one `FabricUIManager.onRequestEventBeat` call, `EventBeat::induce` is called once. And there is a chance for race condition.

Here is a simplified implementation of `induce`. This method may be called many times in sequence. The caller will set [isRequested_](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp#L25) and then invoke [FabricUIManager.onRequestEventBeat](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.cpp#L43). Notice how `FabricUIManager.onRequestEventBeat` is debounced if `isRequested_` flag is true.

```
void EventBeat::induce() const {
  if (!isRequested_ || isBeatCallbackScheduled_) {
   // isRequested_ is not set to false in case isBeatCallbackScheduled_) is true.
    return;
  }

  isRequested_ = false;
  isBeatCallbackScheduled_ = true;

  auto beat = std::function<void(jsi::Runtime&)>(
    // on JS queue
    isBeatCallbackScheduled_ = false;
    // beatCallback_(runtime)
  }

  runtimeScheduler_.scheduleWork(std::move(beat));
}
```

This can get into a state where `isRequested_` is not reset back to false even though `EventBeat::induce` is called when `isBeatCallbackScheduled_` is true.

`AndroidEventBeat::request` -> `isRequested_` is set to true -> `FabricUIManager::onRequestEventBeat` -> `EventBeat::induce` -> `isRequested_` is set to false -> `isBeatCallbackScheduled_` is set to true -> `AndroidEventBeat::request` -> `FabricUIManager::onRequestEventBeat` -> `EventBeat::induce` (early exit because `isBeatCallbackScheduled_` is true) -> `beat` is executed on the JS thread.

From this point on, subsequent calls to `AndroidEventBeat::request` are always debounced because flag `isRequested_` is true.

Any subsequent event on Android will end up calling `EventBeat::induce` and the mechanism gets unstuck.

# The fix

The fix is simple, any time `EventBeat::induce` is called, make sure `request_` flag is set to false. This then satisfied the expectation of `useOptimizedEventBatchingOnAndroid` optimisation.

Reviewed By: rubennorte

Differential Revision: D65566258

fbshipit-source-id: 5f15da8f5cb722b329f9f72b9ddca8e2cac04144
This commit is contained in:
Samuel Susla 2024-11-11 07:01:53 -08:00 committed by Facebook GitHub Bot
parent d7cb6d95a8
commit 685facfd53
3 changed files with 14 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/react-native/ReactAndroid/src/main/jni/react/fabric/AndroidEventBeat.cpp
View File

@ -33,7 +33,7 @@ void AndroidEventBeat::tick() const {
}
void AndroidEventBeat::request() const {
bool alreadyRequested = isRequested_;
bool alreadyRequested = isEventBeatRequested_;
EventBeat::request();
if (!alreadyRequested) {
// Notifies java side that an event will be dispatched (e.g. LayoutEvent)

11
packages/react-native/ReactCommon/react/renderer/core/EventBeat.cpp
View File

@ -22,7 +22,7 @@ void EventBeat::request() const {
react_native_assert(
beatCallback_ &&
"Unexpected state: EventBeat::setBeatCallback was not called before EventBeat::request.");
isRequested_ = true;
isEventBeatRequested_ = true;
}
void EventBeat::requestSynchronous() const {
@ -38,11 +38,16 @@ void EventBeat::setBeatCallback(BeatCallback beatCallback) {
}
void EventBeat::induce() const {
if (!isRequested_ || isBeatCallbackScheduled_) {
if (!isEventBeatRequested_) {
return;
}
isEventBeatRequested_ = false;
if (isBeatCallbackScheduled_) {
return;
}
isRequested_ = false;
isBeatCallbackScheduled_ = true;
auto beat = std::function<void(jsi::Runtime&)>(

6
packages/react-native/ReactCommon/react/renderer/core/EventBeat.h
View File

@ -130,7 +130,11 @@ class EventBeat {
BeatCallback beatCallback_;
std::shared_ptr<OwnerBox> ownerBox_;
mutable std::atomic<bool> isRequested_{false};
/*
* Indicates if event beat was requested to avoid redundant requests.
*/
mutable std::atomic<bool> isEventBeatRequested_{false};
private:
RuntimeScheduler& runtimeScheduler_;