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

Memory leak in hybrid app #11628

Closed
dbof10 opened this issue Dec 26, 2016 · 5 comments
Closed

Memory leak in hybrid app #11628

dbof10 opened this issue Dec 26, 2016 · 5 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@dbof10
Copy link

dbof10 commented Dec 26, 2016

I'm currently using React Native for our new homepage. After having added LeakCanary, I got a log below
https://gist.github.com/dbof10/8d961f7c16bf6056867c2d3940c77011#file-leakstacktrace.

My Activity is
https://gist.github.com/dbof10/8d961f7c16bf6056867c2d3940c77011#file-mainactivity-java

@Trikke
Copy link

Trikke commented Jan 5, 2017

i'm having the same issue, any updates on this?

@konark-hike
Copy link

konark-hike commented Feb 9, 2017

@mkonicek @javache @satya164 Have attached screenshot for the memory leak checked on getting heap dump for same issue. Can someone please suggest a way to get rid of this strong reference!!
Memory Leak GC root path

@hramos
Copy link
Contributor

hramos commented Jul 20, 2017

Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally!

If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:

  • Does the issue still reproduce on the latest release candidate? Post a comment with the version you tested.
  • If so, is there any information missing from the bug report? Post a comment with all the information required by the issue template.
  • Is there a pull request that addresses this issue? Post a comment with the PR number so we can follow up.

If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution.

@hramos hramos closed this as completed Jul 20, 2017
@mkonicek
Copy link
Contributor

mkonicek commented Jan 31, 2018

@Trikke @konark-hike So sorry I missed the cc on this a long time ago! We were overloaded with GitHub issues. Some of them were questions, some potentially more serious like this one and we struggled to go through all issues and reply to all of them.

I found something that might be related:
https://github.com/wix/react-native-navigation/issues/1807

AirBnb, Instagram (blog post) and other hybrid apps use React Native on Android so I think if this was a real issue that happens in production it would have been fixed by now.

However, I just saw reports from LeakCanary when working on my hybrid app just now. I will update here if I find out more.

@mkonicek
Copy link
Contributor

mkonicek commented Feb 1, 2018

Hi @dbof10, @Trikke, @konark-hike,

I added a JS screen to my native app, have been reloading JS, navigating away from that screen and back to it many times and memory usage stays flat, as shown by Android Studio:

screen shot 2018-02-01 at 18 45 28

I'm using LeakCanary in the app which occasionally shows "Dumping memory, app will freeze. Brr!" and writes a heap dump to disk.

After a while I get a notification saying "Leak analysis failed" and the reason it failed is an OOM in the heap analyzer: square/leakcanary#484

I can't see your original gists anymore but from the memory usage graph above my app looks fine.

Here is my Activity (in Kotlin):

class MyActivity : AppCompatActivity(), DefaultHardwareBackBtnHandler {

    private var delegate = MyReactActivityDelegate(this, "MyJsApp")

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        delegate.onCreate(savedInstanceState)  // Run the JS app
    }

    override fun onPause() {
        super.onPause()
        delegate.onPause()
    }

    override fun onResume() {
        super.onResume()
        delegate.onResume()
    }

    override fun onDestroy() {
        super.onDestroy()
        delegate.onDestroy()
    }

    public override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent) {
        delegate.onActivityResult(requestCode, resultCode, data)
    }

    override fun onKeyUp(keyCode: Int, event: KeyEvent): Boolean {
        return delegate.onKeyUp(keyCode, event) || super.onKeyUp(keyCode, event)
    }

    override fun onBackPressed() {
        if (!delegate.onBackPressed()) {
            super.onBackPressed()
        }
    }

    override fun invokeDefaultOnBackPressed() {
        super.onBackPressed()
    }

    public override fun onNewIntent(intent: Intent) {
        if (!delegate.onNewIntent(intent)) {
            super.onNewIntent(intent)
        }
    }
}

I'm using all the standard handling from the ReactActivityDelegate. Just exposing its methods:

class MyReactActivityDelegate(activity: Activity, mainComponentName: String)
    : ReactActivityDelegate(activity, mainComponentName) {

    public override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
    }

    public override fun onPause() {
        super.onPause()
    }

    public override fun onResume() {
        super.onResume()
    }

    public override fun onDestroy() {
        super.onDestroy()
    }
}

And in my Application class, define a host to hold the React Native instance. The ReactActivityDelegate will use that host, via the Activity we passed to it:

/**
 * Simple holder object, cheap to initialise.
 * On first run of JS (call to `getReactInstanceManager`) starts up the JS instance, loads JS from disk,
 * and holds the instance in memory. All the memory can be reclaimed using [{@link MyReactNativeHost#clear()}.
 * It's a good idea to reclaim the memory when the process is running low on memory
 * or when the app is in the background - this reduces chances the system will kill the app.
 */
private MyReactNativeHost reactNativeHost = new ReactNativeHost(this);

@Override public ReactNativeHost getReactNativeHost() {
    return reactNativeHost;
}

And finally the host is quite simple:

class MyReactNativeHost(application: Application) : ReactNativeHost(application) {

    override fun getUseDeveloperSupport(): Boolean {
        return BuildConfig.DEBUG
    }

    override fun getPackages(): List<ReactPackage> {
        return mutableListOf(
                MainReactPackage(),
                MyReactPackageWithAdditionalModules())
    }

    override fun getJSMainModuleName(): String {
        return "index"   // JS filename, without extension
    }
}

@facebook facebook locked as resolved and limited conversation to collaborators Jul 20, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

6 participants