Facebook/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2025-11-01 09:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Make sure FabricUIManager doesn't double-free Binding, or double-destroy itself

Browse Source 
Summary:
While unlikely, in exceptional cases it's possible that `onCatalystInstanceDestroy` is called multiple times, especially since there's no restrictions as to which thread this is called on (currently it's only called on the JS thread from CatalystInstanceImpl) and no synchronization. Make sure it tears down cleanly if called multiple times, and report soft exceptions if that's detected.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D18001678

fbshipit-source-id: 4a7c37b5b3c2207ba1dd9c7d85690391f799518d
This commit is contained in:
Joshua Gross
2019-10-19 02:27:35 -07:00
committed by Facebook Github Bot
parent 95a43f3366
commit f8bf65b8fd
1 changed files with 21 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java
+21 -6
View File
@@ -34,6 +34,7 @@ import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.bridge.ReactMarker;
import com.facebook.react.bridge.ReactMarkerConstants;
import com.facebook.react.bridge.ReactSoftException;
import com.facebook.react.bridge.ReadableArray;
import com.facebook.react.bridge.ReadableMap;
import com.facebook.react.bridge.UIManager;
@@ -123,6 +124,12 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
@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.
*/
private volatile boolean mDestroyed = false;
private long mRunStartTime = 0l;
private long mBatchedExecutionTime = 0l;
private long mDispatchViewUpdatesTime = 0l;
@@ -220,6 +227,19 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
public void onCatalystInstanceDestroy() {
FLog.i(TAG, "FabricUIManager.onCatalystInstanceDestroy");
if (mDestroyed) {
ReactSoftException.logSoftException(
FabricUIManager.TAG, new IllegalStateException("Cannot double-destroy FabricUIManager"));
return;
}
mDestroyed = true;
// 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;
mEventDispatcher.removeBatchEventDispatchedListener(mEventBeatManager);
mEventDispatcher.unregisterEventEmitter(FABRIC);
@@ -229,11 +249,6 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
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.
mIsMountingEnabled = false;
mBinding.unregister();
mBinding = null;
@@ -700,7 +715,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
@Override
public void doFrameGuarded(long frameTimeNanos) {
if (!mIsMountingEnabled) {
if (!mIsMountingEnabled || mDestroyed) {
FLog.w(
ReactConstants.TAG,
"Not flushing pending UI operations because of previously thrown Exception");