Memory leak in Fragment

Question

I am using the LeakCanary library to monitor memory leaks in my app. I received this memory leak and not sure how to track down what is causing it.

05-09 09:32:14.731  28497-31220/? D/LeakCanary﹕ In com.etiennelawlor.minesweeper:0.0.21:21.
    * com.etiennelawlor.minesweeper.fragments.MinesweeperFragment has leaked:
    * GC ROOT com.google.android.gms.games.internal.GamesClientImpl$PopupLocationInfoBinderCallbacks.zzahO
    * references com.google.android.gms.games.internal.PopupManager$PopupManagerHCMR1.zzajo
    * references com.google.android.gms.games.internal.GamesClientImpl.mContext
    * references com.etiennelawlor.minesweeper.activities.MinesweeperActivity.mFragments
    * references android.app.FragmentManagerImpl.mAdded
    * references java.util.ArrayList.array
    * references array java.lang.Object[].[0]
    * leaks com.etiennelawlor.minesweeper.fragments.MinesweeperFragment instance
    * Reference Key: 2f367393-6dfd-4797-8d85-7ac52c431d07
    * Device: LGE google Nexus 5 hammerhead
    * Android Version: 5.1 API: 22
    * Durations: watch=5015ms, gc=141ms, heap dump=1978ms, analysis=23484ms

This is my repo : https://github.com/lawloretienne/Minesweeper

This seems to be an elusive one. I set up an Interface to communicate between a Fragment and an Activity. I set this mCoordinator Interface variable up in onAttach() then I realized I was not nulling it out in onDetach(). I fixed that issue but still am getting a memory leak. Any ideas?

Update

I disabled the Fragment leak watching, and I still get a notification about the activity leaking with the following leak trace :

05-09 17:07:33.074  12934-14824/? D/LeakCanary﹕ In com.etiennelawlor.minesweeper:0.0.21:21.
    * com.etiennelawlor.minesweeper.activities.MinesweeperActivity has leaked:
    * GC ROOT com.google.android.gms.games.internal.GamesClientImpl$PopupLocationInfoBinderCallbacks.zzahO
    * references com.google.android.gms.games.internal.PopupManager$PopupManagerHCMR1.zzajo
    * references com.google.android.gms.games.internal.GamesClientImpl.mContext
    * leaks com.etiennelawlor.minesweeper.activities.MinesweeperActivity instance
    * Reference Key: f4d06830-0e16-43a2-9750-7e2cb77ae24d
    * Device: LGE google Nexus 5 hammerhead
    * Android Version: 5.1 API: 22
    * Durations: watch=5016ms, gc=164ms, heap dump=3430ms, analysis=39535ms

I'd be curious to know if only the fragments leak, or if the activity leaks as well. What if you disable the fragment leak watching, do you still get a notification about the activity leaking? — Pierre-Yves Ricau, May 09 '15 at 20:31
It seems that you need to call GoogleApiClient .unregisterConnectionCallbacks and GoogleApiClient .unregisterConnectionFailedListener method, due to add listener with GoogleApiClient.Builder. https://developer.android.com/reference/com/google/android/gms/common/api/GoogleApiClient.Builder.html#addConnectionCallbacks(com.google.android.gms.common.api.GoogleApiClient.ConnectionCallbacks) — baroqueworksdev, May 10 '15 at 17:27
@baroqueworksdev I unregistered the callback and listener and nulled out the GoogleApiClient in `onDestroy()`. But the activity still has that same memory leak. — Etienne Lawlor, May 11 '15 at 00:29
Can you provide a link to a heap dump after that update (where the activity leaks) ?\ — Pierre-Yves Ricau, May 11 '15 at 22:07
When I click `Share heap dump` in the overflow menu, any app that I try to share it in fails. For example, in GMail it says `Permission denied for attachment`. — Etienne Lawlor, May 11 '15 at 22:22
Consider adding logging to see if your `onDestroy()` method is getting called. If it's not, as Dmide suggested below, you should put your unregister calls in `onStop()` or `onPause()` — TBridges42, May 12 '15 at 17:14
I have the same problem. I unregistered the callbacks in onStop function of the activity. But I still have the problem. Please share it if you solved it. Thank you. — tasomaniac, Aug 02 '15 at 17:17
It is confirmed that Google play service's v8.1.0's location listener is leaking the activity or service it is attached to https://github.com/googlesamples/android-play-location/issues/26 so maybe this is similar? — Marian Paździoch, Oct 29 '15 at 11:46

score 9 · Answer 1 · answered May 15 '15 at 17:09

The documentation indicates that it is safe to call connect() even if the state is "connected" or "connecting". It also indicates that you can safely call disconnect() no matter what the connection state is. Therefore, I would remove the "if" statements around the calls to connect() and disconnect(). However, I doubt that that will make this "leak" go away.

It is clear that GamesClientImpl is storing a reference to your Activity as a Context. I imagine this is occuring in the construction of the GoogleApiClient that happens when you call GoogleApiClient.Builder.build(). It seems like a bug to me that the instance of GoogleApiClient is still around after your Activity has finished. However, if you are supposed to call connect() in onStart() and disconnect() in onStop() that seems to imply that you can reuse the connection (since onStart() and onStop() can be called repeatedly). For this to work, the GoogleApiClient must keep the reference to your Context even after you've called disconnect().

You could try using the global application context instead of your Activity context when creating the GoogleApiClient, as the global application context lives forever (until the process is killed). This should make your "leak" go away:

// Create the Google Api Client with access to Plus and Games
mGoogleApiClient = new GoogleApiClient.Builder(getApplicationContext())
    .addConnectionCallbacks(this)
    .addOnConnectionFailedListener(this)
    .addApi(Plus.API).addScope(Plus.SCOPE_PLUS_LOGIN)
    .addApi(Games.API).addScope(Games.SCOPE_GAMES)
    .build();

Pirdad Sakhizada · Answer 2 · 2015-05-27T18:04:33.710

05-27 13:15:04.478  24415-25236/com.package D/LeakCanary﹕ In com.package:0.0.52-dev:202.
* com.package.launcher.LauncherActivity has leaked:
* GC ROOT com.google.android.gms.ads.internal.request.q.a
* references com.google.android.gms.ads.internal.request.m.d
* references com.google.android.gms.ads.internal.request.c.a
* references com.google.android.gms.ads.internal.j.b
* references com.google.android.gms.ads.internal.aa.f
* references com.google.android.gms.ads.internal.ab.mParent
* references com.google.android.gms.ads.doubleclick.PublisherAdView.mParent
* references android.widget.FrameLayout.mContext
* leaks com.package.launcher.LauncherActivity instance
* Reference Key: 9ba3c5ea-2888-4677-9cfa-ebf38444c994
* Device: LGE google Nexus 5 hammerhead
* Android Version: 5.1.1 API: 22
* Durations: watch=5128ms, gc=150ms, heap dump=5149ms, analysis=29741ms

I was using gms ads library and there was a similar leak. So I fixed the above case by handling it onDestroyView() of my fragment.

@Override
public void onDestroyView() {

    if (mAdView != null) {
        ViewParent parent = mAdView.getParent();
        if (parent != null && parent instanceof ViewGroup) {
            ((ViewGroup) parent).removeView(mAdView);
        }
    }
    super.onDestroyView();
}

So here I'm basically removing my PublisherAdView from it's parent on onDestroyView().

Also, note that I had to use Application Context when I created the PublisherAdView otherwise I would get the following leak:

05-27 13:59:23.684  10041-11496/com.package D/LeakCanary﹕ In com.package:0.0.52-dev:202.
* com.package.launcher.LauncherActivity has leaked:
* GC ROOT com.google.android.gms.ads.internal.request.q.a
* references com.google.android.gms.ads.internal.request.m.b
* leaks com.package.launcher.LauncherActivity instance
* Reference Key: 5acaa61a-ea04-430a-b405-b734216e7e80
* Device: LGE google Nexus 5 hammerhead
* Android Version: 5.1.1 API: 22
* Durations: watch=7275ms, gc=138ms, heap dump=5260ms, analysis=22447ms

Not sure if it's going to solve the question above directly but hope it helps.

score 0 · Answer 3 · edited May 23 '17 at 11:53

0

You shouldn't rely on onDestroy() callback execution, in some cases it might not be called. More solid solution is to put your register/unregister code inside onResume()/onPause().

Same goes (for another reason, of course) for Fragment's onDetach(), move your sensible code to onStop() or onPause().

edited May 23 '17 at 11:53

Community

1
1

answered May 12 '15 at 12:12

Dmide

6,422
3
24
31

7

This is not the case. `onDestroy` can be not called because the only reason: Process was killed by low memory killer. I believe that it doesn't matter whether you unsubscribed from Google Play Services before process death. – pepyakin May 13 '15 at 19:11
I don't imply that this is exatcly the reason behind OP's problem, but these "quirks" of SDK are things to be aware of and maybe it could be related to OP's problem. – Dmide May 17 '15 at 17:10
@Dmide You are wrong, the only quirk is SDK documentation which states what is not true, see this https://commonsware.com/blog/2011/10/03/activities-not-destroyed-to-free-heap-space.html. And don't listen to all what SO brings, e.g. this is bullcrap http://stackoverflow.com/questions/18361719/android-activity-ondestroy-is-not-always-called-and-if-called-only-part-of-the. – Marian Paździoch Oct 29 '15 at 11:41

score 0 · Answer 4 · answered Jan 19 '17 at 12:57

In my case, I had the following code:

googleApiClient = new GoogleApiClient.Builder(activity.getApplicationContext())
            .addConnectionCallbacks(this)
            .addOnConnectionFailedListener(this)
            .addApi(Games.API).addScope(Games.SCOPE_GAMES)
            .build();

The issue were the calls addConnectionCallbacks(this) and addConnectionCallbacks(this). The class referenced by this was keeping a reference to an activity, and as GoogleApiClient won't let go of the references to the connection callbacks/listener, this was resulting in a memory leak.

My solution was to register/unregister the callbacks as the googleApiClient connects/disconnects:

public void connect() {
    mGoogleApiClient.registerConnectionCallbacks(this);
    mGoogleApiClient.registerConnectionFailedListener(this);
    mGoogleApiClient.connect();
}

public void disconnect() {
    if (mGoogleApiClient.isConnected()) {
        mGoogleApiClient.disconnect();
        mGoogleApiClient.unregisterConnectionCallbacks(this);
        mGoogleApiClient.unregisterConnectionFailedListener(this);
    }
}

Memory leak in Fragment

Update

4 Answers4