Skip to content

Commit

Permalink
Fix FpsDebugFrameCallback so that we properly cancel frame loop to av…
Browse files Browse the repository at this point in the history
…oid 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
  • Loading branch information
Xin Chen authored and facebook-github-bot committed Sep 8, 2023
1 parent eb5e7b2 commit a63b443
Showing 1 changed file with 9 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ public FpsInfo(
private final UIManagerModule mUIManagerModule;
private final DidJSUpdateUiDuringFrameDetector mDidJSUpdateUiDuringFrameDetector;

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

@Override
public void doFrame(long l) {
if (mShouldStop) {
return;
}

if (mFirstFrameTime == -1) {
mFirstFrameTime = l;
}
Expand Down Expand Up @@ -124,7 +119,6 @@ public void doFrame(long l) {
}

public void start() {
mShouldStop = false;
mReactContext
.getCatalystInstance()
.addBridgeIdleDebugListener(mDidJSUpdateUiDuringFrameDetector);
Expand All @@ -147,11 +141,19 @@ public void startAndRecordFpsAtEachFrame() {
}

public void stop() {
mShouldStop = true;
mReactContext
.getCatalystInstance()
.removeBridgeIdleDebugListener(mDidJSUpdateUiDuringFrameDetector);
mUIManagerModule.setViewHierarchyUpdateDebugListener(null);
final FpsDebugFrameCallback fpsDebugFrameCallback = this;
UiThreadUtil.runOnUiThread(
new Runnable() {
@Override
public void run() {
mChoreographer = ChoreographerCompat.getInstance();
mChoreographer.removeFrameCallback(fpsDebugFrameCallback);
}
});
}

public double getFPS() {
Expand Down

0 comments on commit a63b443

Please sign in to comment.