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

Android: After the app reloads, we're stuck with a dead thread because of SingleThreadAsserter #18413

Closed
SudoPlz opened this issue Mar 16, 2018 · 10 comments
Labels
Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. Resolution: Locked This issue was locked by the bot.

Comments

@SudoPlz
Copy link
Contributor

SudoPlz commented Mar 16, 2018

Environment

Environment:
OS: macOS Sierra 10.12.6
Node: 8.4.0
Yarn: 1.5.1
npm: 5.3.0
Watchman: 4.7.0
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: 3.0 AI-171.4443003

Packages: (wanted => installed)
react: 16.2.0 => 16.2.0
react-native: 0.53.0 => 0.53.0

Expected Behavior

I would expect the SingleThreadAsserter to clear the mThread value after the app reloads, so that a new thread can be used on every assertion from that point in.

Actual Behavior

Instead, the SingleThreadAsserter locks to the thread it was created with, so when the app reloads, and that thread dies we're stuck with a dead thread, which causes all subsequent assertions to fail.
Almost every uiManager method requires that assertion, so that's a big issue.

Steps to Reproduce

Here's a repo that suffers from that issue.
https://github.com/SudoPlz/react-native-synchronous-list

  1. Download
  2. cd RNExample
  3. yarn install
  4. Run the android project.

All works well until you reload. After the reload we get:

FATAL EXCEPTION: main
 Process: com.rnexample, PID: 8519
 java.lang.AssertionError
     at com.facebook.infer.annotation.Assertions.assertCondition(Assertions.java:66)
     at com.facebook.react.common.SingleThreadAsserter.assertNow(SingleThreadAsserter.java:27)
     at com.facebook.react.uimanager.ShadowNodeRegistry.getNode(ShadowNodeRegistry.java:67)
     at com.facebook.react.uimanager.UIImplementation.createView(UIImplementation.java:278)
     at com.facebook.react.uimanager.UIManagerModule.createView(UIManagerModule.java:364)
     at com.sudoplz.rnsynchronouslistmanager.Views.SyncRootView$3.run(SyncRootView.java:172)
     at android.os.Handler.handleCallback(Handler.java:751)
     at android.os.Handler.dispatchMessage(Handler.java:95)
     at android.os.Looper.loop(Looper.java:154)
     at android.app.ActivityThread.main(ActivityThread.java:6119)
     at java.lang.reflect.Method.invoke(Native Method)
     at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)
@react-native-bot react-native-bot added Android Ran Commands One of our bots successfully processed a command. labels Mar 16, 2018
@react-native-bot
Copy link
Collaborator

Thanks for posting this! It looks like your issue may refer to an older version of React Native. Can you reproduce the issue on the latest release, v0.54.0?

Thank you for your contributions.

How to ContributeWhat to Expect from Maintainers

@react-native-bot react-native-bot added Old Version ⏪ Ran Commands One of our bots successfully processed a command. labels Mar 16, 2018
@SudoPlz
Copy link
Contributor Author

SudoPlz commented Mar 16, 2018

Same exact issue on 0.54.0 yes.

@react-native-bot react-native-bot added the Missing Test Plan This PR appears to be missing a test plan. label Mar 16, 2018
@hramos hramos removed the Missing Test Plan This PR appears to be missing a test plan. label Mar 16, 2018
@CodeRabbitYu
Copy link

same this question

@react-native-bot react-native-bot added Android Ran Commands One of our bots successfully processed a command. labels Mar 18, 2018
@react-native-bot react-native-bot added Platform: Android Android applications. Ran Commands One of our bots successfully processed a command. labels Mar 18, 2018
@react-native-bot react-native-bot added the Platform: macOS Building on macOS. label Mar 20, 2018
@SudoPlz
Copy link
Contributor Author

SudoPlz commented Mar 21, 2018

Since I don't see anyone responding to this, I'll try to use reflection to change the mThread value to null locally.
That's definitely a hack but I have no clue what else I can do.

https://github.com/wix/react-native-navigation/blob/8b034b2ebb95f482b594a467bd98942f0bce16ce/android/app/src/main/java/com/reactnativenavigation/utils/ReflectionUtils.java#L9

@hramos hramos removed the Platform: macOS Building on macOS. label Mar 29, 2018
@SudoPlz
Copy link
Contributor Author

SudoPlz commented Apr 26, 2018

Anyone? What is even the purpose of the SingleThreadAsserter ??

@pocketNelson
Copy link

I've been integrating ReactNative into an existing project that is mainly built on Fragments. I'm experiencing this using the react-native-android-fragment library. Yes, any kind of reload kicks the fragment off the thread. I guess, I can just try to listen for this failure then call the ReactFragment hosting my material again?

@janicduplessis
Copy link
Contributor

@SudoPlz The goal of this class is to validate that some code is always run from the same thread. I guess a proper fix would be to add a reset method and call it when the native modules that use it gets invalidated (onCatalystInstanceDestroy method).

If you'd like to open up a PR to fix it that would be great.

@SudoPlz
Copy link
Contributor Author

SudoPlz commented Aug 30, 2018

I'm not sure I know how to do that @janicduplessis do you mind elaborating a bit more? I'd be grateful.

@janicduplessis
Copy link
Contributor

I had a quick look at the code and is it possible that you are holding on to an old instance of ReactApplicationContext? I think you should use ReactInstanceManager##getCurrentReactContext to make sure you have the proper one like the code in ReactRootView does.

You could override

to get the ReactInstanceManager since it is private in ReactRootView sadly.

@SudoPlz
Copy link
Contributor Author

SudoPlz commented Aug 30, 2018

Got it, always requesting the current react application context fixed the issue, thank you so much @janicduplessis !

@SudoPlz SudoPlz closed this as completed Aug 30, 2018
@facebook facebook locked as resolved and limited conversation to collaborators Aug 30, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Platform: Android Android applications. 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

6 participants