From d5e3f081c6b41697533775d378969fcf554c7290 Mon Sep 17 00:00:00 2001 From: Sergei Dryganets Date: Thu, 11 Jan 2018 18:50:25 -0800 Subject: [PATCH] Fix #6228: Crash in Android dialog module. Summary: Android dialog module has a race condition as a result of which it crashes. See this issue: https://github.com/facebook/react-native/issues/6228. The mIsInForeground flag is set on UI thread but was used from the ReactMethod thread. Now all public methods of FragmentManagerHelper called from UI thread only. Asserts are added in appropriate places to prevent future regressions. Make sure that dialogs work after this change. It will be nearly impossible to reproduce the issue manually but automatic regression tests should be able to catch this. At least our tests were crashing on some dialog scenarios from time to time. [ANDROID] [MINOR] [BUGFIX] - Race condition fix in Android Dialogs module. Closes https://github.com/facebook/react-native/pull/17392 Reviewed By: achen1 Differential Revision: D6708787 Pulled By: mdvacca fbshipit-source-id: 99beb3ea3046286cc973f7677e98ff36f162b09b --- .../react/modules/dialog/DialogModule.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java index d543b6cf083..6c2f39d180d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/dialog/DialogModule.java @@ -28,6 +28,7 @@ import com.facebook.react.bridge.ReactContextBaseJavaModule; import com.facebook.react.bridge.ReactMethod; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; +import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.common.MapBuilder; import com.facebook.react.module.annotations.ReactModule; @@ -95,6 +96,7 @@ public class DialogModule extends ReactContextBaseJavaModule implements Lifecycl } public void showPendingAlert() { + UiThreadUtil.assertOnUiThread(); if (mFragmentToShow == null) { return; } @@ -123,6 +125,8 @@ public class DialogModule extends ReactContextBaseJavaModule implements Lifecycl } public void showNewAlert(boolean isInForeground, Bundle arguments, Callback actionCallback) { + UiThreadUtil.assertOnUiThread(); + dismissExisting(); AlertFragmentListener actionListener = @@ -218,8 +222,8 @@ public class DialogModule extends ReactContextBaseJavaModule implements Lifecycl public void showAlert( ReadableMap options, Callback errorCallback, - Callback actionCallback) { - FragmentManagerHelper fragmentManagerHelper = getFragmentManagerHelper(); + final Callback actionCallback) { + final FragmentManagerHelper fragmentManagerHelper = getFragmentManagerHelper(); if (fragmentManagerHelper == null) { errorCallback.invoke("Tried to show an alert while not attached to an Activity"); return; @@ -253,7 +257,13 @@ public class DialogModule extends ReactContextBaseJavaModule implements Lifecycl args.putBoolean(KEY_CANCELABLE, options.getBoolean(KEY_CANCELABLE)); } - fragmentManagerHelper.showNewAlert(mIsInForeground, args, actionCallback); + UiThreadUtil.runOnUiThread(new Runnable() { + @Override + public void run() { + fragmentManagerHelper.showNewAlert(mIsInForeground, args, actionCallback); + } + }); + } /**