Skip to content

Commit

Permalink
Fix #6228: Crash in Android dialog module.
Browse files Browse the repository at this point in the history
Summary:
Android dialog module has a race condition as a result of which it crashes.
See this issue:
#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 #17392

Reviewed By: achen1

Differential Revision: D6708787

Pulled By: mdvacca

fbshipit-source-id: 99beb3ea3046286cc973f7677e98ff36f162b09b
  • Loading branch information
dryganets authored and facebook-github-bot committed Jan 12, 2018
1 parent 06ebaf2 commit d5e3f08
Showing 1 changed file with 13 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
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;

Expand Down Expand Up @@ -95,6 +96,7 @@ public FragmentManagerHelper(android.app.FragmentManager fragmentManager) {
}

public void showPendingAlert() {
UiThreadUtil.assertOnUiThread();
if (mFragmentToShow == null) {
return;
}
Expand Down Expand Up @@ -123,6 +125,8 @@ private void dismissExisting() {
}

public void showNewAlert(boolean isInForeground, Bundle arguments, Callback actionCallback) {
UiThreadUtil.assertOnUiThread();

dismissExisting();

AlertFragmentListener actionListener =
Expand Down Expand Up @@ -218,8 +222,8 @@ public void onHostResume() {
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;
Expand Down Expand Up @@ -253,7 +257,13 @@ public void showAlert(
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);
}
});

}

/**
Expand Down

0 comments on commit d5e3f08

Please sign in to comment.