From 36ab9f6a602060327c8f9af666b9174c2fba0a87 Mon Sep 17 00:00:00 2001 From: Andy Street Date: Fri, 10 Feb 2017 06:49:38 -0800 Subject: [PATCH] Don't continue to execute UI operations after one has thrown an exception Summary: If we throw an exception from a UIOperation, the UI is likely in a bad state and we shouldn't execute any other operations on it. In non-dev mode, this would mean we've crashed and don't have to worry about that. But in dev mode, we may have shown a redbox and the instance is still active. This means doing something like reloading, which will trigger onPause and thus flush more UI operations, can crash the app while its sitting on a red box. This diff aims to prevent that by no longer executing UI operations after one has thrown an exception. Reviewed By: AaaChiuuu Differential Revision: D4536925 fbshipit-source-id: 15c21bb76ad3419a54d9d5de94b6bd1f70a3e4a4 --- .../react/uimanager/UIViewOperationQueue.java | 27 +++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java index 938aaf88c..c7cd96255 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java @@ -17,6 +17,7 @@ import java.util.ArrayList; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import com.facebook.common.logging.FLog; import com.facebook.react.animation.Animation; import com.facebook.react.animation.AnimationRegistry; import com.facebook.react.bridge.Callback; @@ -26,8 +27,8 @@ import com.facebook.react.bridge.SoftAssertions; import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; -import com.facebook.react.bridge.SoftAssertions; import com.facebook.react.bridge.UiThreadUtil; +import com.facebook.react.common.ReactConstants; import com.facebook.react.uimanager.debug.NotThreadSafeViewHierarchyUpdateDebugListener; import com.facebook.systrace.Systrace; import com.facebook.systrace.SystraceMessage; @@ -537,6 +538,7 @@ public class UIViewOperationQueue { private ArrayDeque mNonBatchedOperations = new ArrayDeque<>(); private @Nullable NotThreadSafeViewHierarchyUpdateDebugListener mViewHierarchyUpdateDebugListener; private boolean mIsDispatchUIFrameCallbackEnqueued = false; + private boolean mIsInIllegalUIState = false; public UIViewOperationQueue( ReactApplicationContext reactContext, @@ -792,6 +794,9 @@ public class UIViewOperationQueue { if (mViewHierarchyUpdateDebugListener != null) { mViewHierarchyUpdateDebugListener.onViewHierarchyUpdateFinished(); } + } catch (Exception e) { + mIsInIllegalUIState = true; + throw e; } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } @@ -827,6 +832,12 @@ public class UIViewOperationQueue { } private void flushPendingBatches() { + if (mIsInIllegalUIState) { + FLog.w( + ReactConstants.TAG, + "Not flushing pending UI operations because of previously thrown Exception"); + return; + } synchronized (mDispatchRunnablesLock) { for (int i = 0; i < mDispatchUIRunnables.size(); i++) { mDispatchUIRunnables.get(i).run(); @@ -861,6 +872,13 @@ public class UIViewOperationQueue { @Override public void doFrameGuarded(long frameTimeNanos) { + if (mIsInIllegalUIState) { + FLog.w( + ReactConstants.TAG, + "Not flushing pending UI operations because of previously thrown Exception"); + return; + } + Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "dispatchNonBatchedUIOperations"); try { dispatchPendingNonBatchedOperations(frameTimeNanos); @@ -890,7 +908,12 @@ public class UIViewOperationQueue { nextOperation = mNonBatchedOperations.pollFirst(); } - nextOperation.execute(); + try { + nextOperation.execute(); + } catch (Exception e) { + mIsInIllegalUIState = true; + throw e; + } } } }