Random crashes when loading two react bundles

[oximer]

Is this a bug report?

Yes

Have you read the Contributing Guidelines?

Yes

Environment

Environment: OS: macOS Sierra 10.12.6 Node: 8.9.0 Yarn: Not Found npm: 5.6.0 Watchman: 4.9.0 Xcode: Not Found Android Studio: 3.0 AI-171.4443003

Packages: (wanted => installed) react: 16.0.0 => 16.0.0 react-native: 0.51.0 => 0.51.0

Target Platform: Adnroid (27)

Steps to Reproduce

It's difficult to describes steps to reproduce it, because there is a lot of native code here. I basically have a single activity with two bundles on it. I'm using the class ReactInstanceManagerBuilder to create my bundles.

1. Create a simple activity
2. Instantiate a React Bundle
3. Get a RootView from the bundle
4. Inflate this RootView into the page
5. Instantiate another React Bundle
6. Get the RootView from the bundle
7. Inflate the RootView 2 into the page

Expected Behavior

Open two bundles in a single Android Activity.

Actual Behavior

It actually opens it. However, I have random crashes when the bundles tries to update its view props.

public void updateShadowNodeProp(
        ReactShadowNode nodeToUpdate,
        ReactStylesDiffMap props) {
      try {
        if (mIndex == null) {
          SHADOW_ARGS[0] = extractProperty(props);
          mSetter.invoke(nodeToUpdate, SHADOW_ARGS);
          Arrays.fill(SHADOW_ARGS, null);
        } else {
          SHADOW_GROUP_ARGS[0] = mIndex;
          SHADOW_GROUP_ARGS[1] = extractProperty(props);
          mSetter.invoke(nodeToUpdate, SHADOW_GROUP_ARGS);
          Arrays.fill(SHADOW_GROUP_ARGS, null);
        }
      } catch (Throwable t) {
        FLog.e(ViewManager.class, "Error while updating prop " + mPropName, t);
        throw new JSApplicationIllegalArgumentException("Error while updating property '" +
            mPropName + "' in shadow node of type: " + nodeToUpdate.getViewClass(), t);
      }
    }

This method throws many different exceptions.

Example

com.facebook.react.bridge.JSApplicationIllegalArgumentException: Error while updating property 'borderBottomWidth' in shadow node of type: RCTView at com.facebook.react.uimanager.ViewManagersPropertyCache$PropSetter.updateShadowNodeProp(ViewManagersPropertyCache.java:113) at com.facebook.react.uimanager.ViewManagerPropertyUpdater$FallbackShadowNodeSetter.setProperty(ViewManagerPropertyUpdater.java:154) at com.facebook.react.uimanager.ViewManagerPropertyUpdater.updateProps(ViewManagerPropertyUpdater.java:58) at com.facebook.react.uimanager.ReactShadowNodeImpl.updateProperties(ReactShadowNodeImpl.java:298) at com.facebook.react.uimanager.UIImplementation.createView(UIImplementation.java:289) at com.facebook.react.uimanager.UIManagerModule.createView(UIManagerModule.java:373) at java.lang.reflect.Method.invoke(Native Method) at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:374) at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:162) at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method) at android.os.Handler.handleCallback(Handler.java:790) at android.os.Handler.dispatchMessage(Handler.java:99) at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:31) at android.os.Looper.loop(Looper.java:164) at com.facebook.react.bridge.queue.MessageQueueThreadImpl$3.run(MessageQueueThreadImpl.java:194) at java.lang.Thread.run(Thread.java:764)

[oximer]

package com.facebook.react.uimanager;
class ViewManagersPropertyCache {

static abstract class PropSetter {

// The following Object arrays are used to prevent extra allocations from varargs when we call
    // Method.invoke. It's safe for those objects to be static as we update properties in a single
    // thread sequentially
    private static final Object[] VIEW_MGR_ARGS = new Object[2];
    private static final Object[] VIEW_MGR_GROUP_ARGS = new Object[3];
    private static final Object[] SHADOW_ARGS = new Object[1];
    private static final Object[] SHADOW_GROUP_ARGS = new Object[2];

...

I'm guessing that this SHADOW_ARGS are causing the problem, since they are static. The code assumes that you have a single thread sequentially.

It's that true if you have two bundles running? Each one in a different fragment?

In my opinion:

The PropSetter should be per bundle instead of be a static field. Thus, we can avoid extra allocations problems and it will be thread safe.

Another option is to implement a mutex or semaphore, making it really thread safe.

[alexandrelandim]

I'm having the same issue here.

[vocampos]

Has anyone managed to solve it?

[augustoazevedo]

It seems to be the reason my app has been crashing too... Looking forward to a solution!!!

[gutoglup]

this problem is happening frequently with me too

[tbfreitas]

Any solution or workaround for it ? I have tried everything to avoid this, with no success.

[react-native-bot]

Thanks for posting this! It looks like you may not be using the latest version of React Native, v0.53.0, released on January 2018. Can you make sure this issue can still be reproduced in the latest version?

I am going to close this, but please feel free to open a new issue if you are able to confirm that this is still a problem in v0.53.0 or newer.

_{How to Contribute • What to Expect from Maintainers}

[ashutosh-akss]

same issue here with latest bundle downloaded today

[chyi13]

I reolved the problem by changing SHADOW_ARGS and SHADOW_GROUP_ARGS to local variables inside updateShadowNodeProp().

[ashutosh-akss]

@chyi13 can you please post some sample code for this and filename.

[chyi13]

@ashutosh-akss its in ViewManagersPropertyCache.java. But it may not be a good solution due to the commets in this file,

// The following Object arrays are used to prevent extra allocations from varargs when we call // Method.invoke. It's safe for those objects to be static as we update properties in a single // thread sequentially.

[hey99xx]

This is happening in latest RN version. It's not fixed. Can someone reopen this?

@ashutosh-akss The sample code to use local variables is like this:

- SHADOW_ARGS[0] = extractProperty(props);
- mSetter.invoke(nodeToUpdate, SHADOW_ARGS);
- Arrays.fill(SHADOW_ARGS, null);
+ Object[] local_SHADOW_ARGS = new Object[SHADOW_ARGS.length];
+ local_SHADOW_ARGS[0] = extractProperty(props);
+ mSetter.invoke(nodeToUpdate, local_SHADOW_ARGS);
+ Arrays.fill(local_SHADOW_ARGS, null);

You need to do repeat this sort of changes in https://github.com/facebook/react-native/blob/0.56-stable/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagersPropertyCache.java#L81-L111

[rosskhanas]

@hey99xx seems like your example actually fixes the issue. Thank you!

[takunee]

@hey99xx Good job! I fixes the issue. Thanks!

anyone can reopen this ?

@react-native-bot

[ZuJunJun]

This is happening in latest RN version. It's not fixed. Can someone reopen this?

@ashutosh-akss The sample code to use local variables is like this:

- SHADOW_ARGS[0] = extractProperty(props);
- mSetter.invoke(nodeToUpdate, SHADOW_ARGS);
- Arrays.fill(SHADOW_ARGS, null);
+ Object[] local_SHADOW_ARGS = new Object[SHADOW_ARGS.length];
+ local_SHADOW_ARGS[0] = extractProperty(props);
+ mSetter.invoke(nodeToUpdate, local_SHADOW_ARGS);
+ Arrays.fill(local_SHADOW_ARGS, null);

You need to do repeat this sort of changes in https://github.com/facebook/react-native/blob/0.56-stable/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManagersPropertyCache.java#L81-L111

How can I modify this file? The compiler I use is Android studio. It prompts me that file is read-only. I import react-native with build. gradle. I edit this file with webstrom. Then I run the project and still report errors. Then I enter the error file in adnroid studio. I see that the code I modified in webstorm is not in the file. Please Thank you for your advice.

[hey99xx]

@ZuJunJun First, stop spamming this thread. I'm happy to reply once but then please delete your 2 unneeded comments.

Your editor being AndroidStudio or WebStorm does not matter. You need to build RN Android code from source, and take a dependency on the generated AAR instead of the one coming from node_modules. The build instructions are at https://facebook.github.io/react-native/docs/building-from-source , searching Google also brings https://medium.com/zegocover/building-react-native-from-source-android-557d21b7b9c3 If you cannot build from source, you cannot make any arbitrary patches mentioned in any GitHub issue, not just this one.

[ZuJunJun]

@ZuJunJun First, stop spamming this thread. I'm happy to reply once but then please delete your 2 unneeded comments.

Your editor being AndroidStudio or WebStorm does not matter. You need to build RN Android code from source, and take a dependency on the generated AAR instead of the one coming from node_modules. The build instructions are at https://facebook.github.io/react-native/docs/building-from-source , searching Google also brings https://medium.com/zegocover/building-react-native-from-source-android-557d21b7b9c3 If you cannot build from source, you cannot make any arbitrary patches mentioned in any GitHub issue, not just this one.

Thank you. First of all, I'm very sorry for my comments. It's urgent. I'm very sorry. Thank you for your reply.

[ZuJunJun]

@ZuJunJun First, stop spamming this thread. I'm happy to reply once but then please delete your 2 unneeded comments.

Your editor being AndroidStudio or WebStorm does not matter. You need to build RN Android code from source, and take a dependency on the generated AAR instead of the one coming from node_modules. The build instructions are at https://facebook.github.io/react-native/docs/building-from-source , searching Google also brings https://medium.com/zegocover/building-react-native-from-source-android-557d21b7b9c3 If you cannot build from source, you cannot make any arbitrary patches mentioned in any GitHub issue, not just this one.

Hello, I have revised two places, but they will still collapse and report the same problem. this file and I hope you can help me. Thank you. and my react-native version is 0.55.4，then my react-native was building-from-source to modify two files;thanks