Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Random crashes when loading two react bundles #17873

Closed
oximer opened this issue Feb 6, 2018 · 18 comments
Closed

Random crashes when loading two react bundles #17873

oximer opened this issue Feb 6, 2018 · 18 comments
Labels
Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot.

Comments

@oximer
Copy link

oximer commented Feb 6, 2018

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
Copy link
Author

oximer commented Feb 6, 2018

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.

@oximer oximer changed the title Constant crashs when loading two react bundles Random crashes when loading two react bundles Feb 6, 2018
@alexandrelandim
Copy link

I'm having the same issue here.

@vocampos
Copy link

vocampos commented Feb 6, 2018

Has anyone managed to solve it?

@augustoazevedo
Copy link

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

@gutoglup
Copy link

gutoglup commented Feb 7, 2018

this problem is happening frequently with me too

@tbfreitas
Copy link

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

@react-native-bot
Copy link
Collaborator

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 ContributeWhat to Expect from Maintainers

@react-native-bot react-native-bot added Ran Commands One of our bots successfully processed a command. Stale There has been a lack of activity on this issue and it may be closed soon. labels Feb 24, 2018
@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Feb 24, 2018
@ashutosh-akss
Copy link

same issue here with latest bundle downloaded today

@chyi13
Copy link

chyi13 commented Jul 9, 2018

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

@ashutosh-akss
Copy link

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

@chyi13
Copy link

chyi13 commented Jul 10, 2018

@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
Copy link

hey99xx commented Oct 5, 2018

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
Copy link

rosskhanas commented Nov 8, 2018

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

@takunee
Copy link

takunee commented Nov 9, 2018

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

anyone can reopen this ?

@react-native-bot

@ZuJunJun
Copy link

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
Copy link

hey99xx commented Dec 11, 2018

@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
Copy link

@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
Copy link

@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.
image
this file and
image
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

@facebook facebook locked as resolved and limited conversation to collaborators Feb 24, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests