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

MixpanelActivityLifecycleCallbacks leaks activities #463

Closed
mindhacker42 opened this issue May 26, 2017 · 10 comments
Closed

MixpanelActivityLifecycleCallbacks leaks activities #463

mindhacker42 opened this issue May 26, 2017 · 10 comments

Comments

@mindhacker42
Copy link

mindhacker42 commented May 26, 2017

Library version: 5.1.0

LeakCanary trace:

In xxxxxxx.simpledemo:1.0:1.
* it.reveel.simpledemo.MainActivity has leaked:
* GC ROOT static it.reveel.sdk.internal.analytics.Analytics.mixpanelApi
* references com.mixpanel.android.mpmetrics.MixpanelAPI.mMixpanelActivityLifecycleCallbacks
* references com.mixpanel.android.mpmetrics.MixpanelActivityLifecycleCallbacks.check
* references com.mixpanel.android.mpmetrics.MixpanelActivityLifecycleCallbacks$1.val$activity (anonymous implementation of java.lang.Runnable)
* leaks xxxxxxx.simpledemo.MainActivity instance

* Retaining: 148 KB.
* Reference Key: 63546fe7-d0af-4992-a355-ca5a90e2ee4c
* Device: LGE google Nexus 5 hammerhead
* Android Version: 6.0.1 API: 23 LeakCanary: 1.5 00f37f5
* Durations: watch=5064ms, gc=130ms, heap dump=4808ms, analysis=236627ms

I looked at the code and I saw that you register for activity lifecycle callbacks to measure session duration and other stuff. You can disable other stuff but you can't disable measuring session duration.
Problem occurs when you capture activity object in Runnable and you retain that in variable and since MixpanelActivityLifecycleCallbacks lives forever in MixpanelAPI singleton that Runnable never gets GCed.

  1. It might help to unregister for lifecycle events in onResume or somewhere since you don't need to listen for that in background.
  2. Otherwise maybe putting "check" variable to null when removing callbacks from handler will also help.
@mindhacker42
Copy link
Author

mindhacker42 commented May 28, 2017

After investigating further I saw that library version 5.0.2 does not leak. Problem is in 5.1.0 in the line 81 of MixpanelActivityLifecycleCallbacks.
Because you use activity passed in onActivityPaused method to get application context you capture activity instance in Runnable named check.

You could get minimumSessionDuration and sessionTimeoutDuration variables in MixpanelActivityLifecycleCallbacks constructor or at least use context from an instance variable or MixpanelAPI instance.

@mwolfe38
Copy link

mwolfe38 commented Jun 5, 2017

I'm seeing this as well. It's causing some major memory leaks in my app. Any progress on a fix?

@JasonSznol
Copy link

+1 on this leak.

@patedit
Copy link
Contributor

patedit commented Jun 6, 2017

Thanks a lot for reporting this @mindhacker42 and thanks @mwolfe38 @JasonSznol for confirming the issue!
I've fixed the problem (3662663) and released version 5.1.2 just now 👍

@JasonSznol
Copy link

Have you published the updated version to a maven repository? Gradle is saying it can't be found and I did not see it in bintray last night.

@patedit
Copy link
Contributor

patedit commented Jun 7, 2017

@JasonSznol Can you try now? Thanks!

@JasonSznol
Copy link

Can confirm it is working, thanks!

@patedit
Copy link
Contributor

patedit commented Jun 8, 2017

Amazing. Thanks for your help @JasonSznol !

@patedit patedit closed this as completed Jun 8, 2017
@fleficher
Copy link

fleficher commented Jan 8, 2018

Hi @patedit i'm having the same issue using the 5.2.1 version :

`* xxxxxxx.MyActivity has leaked:

  • GC ROOT static A4S.b
  • references xxxxxxx.MyApplication.mMixpanelActivityLifecycleCallbacks
  • references ArrayList.elementData
  • references array Object[].[14]
  • references MixpanelActivityLifecycleCallbacks.mCheck
  • references MixpanelActivityLifecycleCallbacks$1.val$activity (anonymous implementation of java.lang.Runnable)
  • leaks xxxxxxx.MyActivity instance
    `

@fleficher
Copy link

Oups, sorry @patedit, we had a class named 'MixpanelActivityLifecycleCallbacks' in our code too which is extremly closed to yours. So it was a problem on my side :)

Wrong alert

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants