Skip to content

Commit

Permalink
Refactor the cancellation of DispatchUIFrameCallback
Browse files Browse the repository at this point in the history
Summary:
This diff refactors the stopping of DispatchUIFrameCallback on FabricUIManager to make it thread safe

Changelog: Refactor the cancellation of dispatching of Mounting operations for Fabric

Reviewed By: JoshuaGross

Differential Revision: D18010922

fbshipit-source-id: 305bc65576698cb785a2a2308cbd03db4a9a97e4
  • Loading branch information
mdvacca authored and facebook-github-bot committed Oct 25, 2019
1 parent 619e27e commit c5321e8
Showing 1 changed file with 17 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import android.annotation.SuppressLint;
import android.os.SystemClock;
import android.view.View;
import androidx.annotation.AnyThread;
import androidx.annotation.GuardedBy;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
Expand Down Expand Up @@ -121,9 +122,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
@NonNull
private final DispatchUIFrameCallback mDispatchUIFrameCallback;

@ThreadConfined(UI)
private volatile boolean mIsMountingEnabled = true;

/**
* This is used to keep track of whether or not the FabricUIManager has been destroyed. Once the
* Catalyst instance is being destroyed, we should cease all operation here.
Expand Down Expand Up @@ -238,17 +236,22 @@ public void onCatalystInstanceDestroy() {
// This is not technically thread-safe, since it's read on the UI thread and written
// here on the JS thread. We've marked it as volatile so that this writes to UI-thread
// memory immediately.
mIsMountingEnabled = false;
mDispatchUIFrameCallback.stop();

mEventDispatcher.removeBatchEventDispatchedListener(mEventBeatManager);
mEventDispatcher.unregisterEventEmitter(FABRIC);

// Remove lifecycle listeners (onHostResume, onHostPause) since the FabricUIManager is going
// away. This and setting `mIsMountingEnabled` to false will cause the choreographer
// away. Then stop the mDispatchUIFrameCallback false will cause the choreographer
// callbacks to stop firing.
mReactApplicationContext.removeLifecycleEventListener(this);
onHostPause();

// This is not technically thread-safe, since it's read on the UI thread and written
// here on the JS thread. We've marked it as volatile so that this writes to UI-thread
// memory immediately.
mDispatchUIFrameCallback.stop();

mBinding.unregister();
mBinding = null;

Expand Down Expand Up @@ -709,10 +712,17 @@ public Map<String, Long> getPerformanceCounters() {

private class DispatchUIFrameCallback extends GuardedFrameCallback {

private DispatchUIFrameCallback(ReactContext reactContext) {
private volatile boolean mIsMountingEnabled = true;

private DispatchUIFrameCallback(@NonNull ReactContext reactContext) {
super(reactContext);
}

@AnyThread
void stop() {
mIsMountingEnabled = false;
}

@Override
public void doFrameGuarded(long frameTimeNanos) {
if (!mIsMountingEnabled || mDestroyed) {
Expand All @@ -730,7 +740,7 @@ public void doFrameGuarded(long frameTimeNanos) {

} catch (Exception ex) {
FLog.i(ReactConstants.TAG, "Exception thrown when executing UIFrameGuarded", ex);
mIsMountingEnabled = false;
stop();
throw ex;
} finally {
ReactChoreographer.getInstance()
Expand Down

0 comments on commit c5321e8

Please sign in to comment.