Skip to content

Commit a63b443

Browse files
Xin Chenfacebook-github-bot
authored andcommitted
Fix FpsDebugFrameCallback so that we properly cancel frame loop to avoid race (#38671)
Summary: Pull Request resolved: #38671 Fix a race condition when we unmount and mount a view using FpsView too frequently. In this case, the frame loop callback didn't get a chance to unset the `mShouldStop` flag, causing the old frame loop continues to run unexpectedly. The fix here guarantees `stop` would queue logic that removes the frame loop callback, and a later `start` would queue logic that attaches a new frame loop callback. Since both of them happens on UI thread, they are in sync. Changelog: [Android][Fixed] - Fix a race with FpsView on using FpsDebugFrameCallback. Reviewed By: hoxyq Differential Revision: D47849848 fbshipit-source-id: 8c4be40e86be128734bfa3f571fd3a1735976c7c
1 parent eb5e7b2 commit a63b443

File tree

1 file changed

+9
-7
lines changed

1 file changed

+9
-7
lines changed

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/debug/FpsDebugFrameCallback.java

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ public FpsInfo(
6363
private final UIManagerModule mUIManagerModule;
6464
private final DidJSUpdateUiDuringFrameDetector mDidJSUpdateUiDuringFrameDetector;
6565

66-
private boolean mShouldStop = false;
6766
private long mFirstFrameTime = -1;
6867
private long mLastFrameTime = -1;
6968
private int mNumFrameCallbacks = 0;
@@ -82,10 +81,6 @@ public FpsDebugFrameCallback(ReactContext reactContext) {
8281

8382
@Override
8483
public void doFrame(long l) {
85-
if (mShouldStop) {
86-
return;
87-
}
88-
8984
if (mFirstFrameTime == -1) {
9085
mFirstFrameTime = l;
9186
}
@@ -124,7 +119,6 @@ public void doFrame(long l) {
124119
}
125120

126121
public void start() {
127-
mShouldStop = false;
128122
mReactContext
129123
.getCatalystInstance()
130124
.addBridgeIdleDebugListener(mDidJSUpdateUiDuringFrameDetector);
@@ -147,11 +141,19 @@ public void startAndRecordFpsAtEachFrame() {
147141
}
148142

149143
public void stop() {
150-
mShouldStop = true;
151144
mReactContext
152145
.getCatalystInstance()
153146
.removeBridgeIdleDebugListener(mDidJSUpdateUiDuringFrameDetector);
154147
mUIManagerModule.setViewHierarchyUpdateDebugListener(null);
148+
final FpsDebugFrameCallback fpsDebugFrameCallback = this;
149+
UiThreadUtil.runOnUiThread(
150+
new Runnable() {
151+
@Override
152+
public void run() {
153+
mChoreographer = ChoreographerCompat.getInstance();
154+
mChoreographer.removeFrameCallback(fpsDebugFrameCallback);
155+
}
156+
});
155157
}
156158

157159
public double getFPS() {

0 commit comments

Comments
 (0)