Anonymous class hidden reference to local variable

Question

I encountered a reported memory leak in my Android app, after some investigation I pretty much find out where the leak is, here is the simplified code:

public class LeakTracker {
    public static List<Callback> callbacks = new ArrayList<>();
    public static List<WeakReference<LeakingActivity>> weakList = new ArrayList<>();

    // causes leak of activity
    public void startLeak(final LeakingActivity activity) {
        callbacks.add(new Callback() {
            // remove this line then no leak
            Wrapper wrapper = new Wrapper(activity);
            @Override
            public void onCall() {
            }
        });
    }

    // no leak here
    public void startLeak2(final LeakingActivity activity) {
        weakList.add(new WeakReference<>(activity));
    }

    public interface Callback {
        void onCall();
    }

    static class Wrapper {
        private WeakReference<LeakingActivity> weakReference;

        public Wrapper(final LeakingActivity activity) {
            weakReference = new WeakReference<LeakingActivity>(activity);
        }
    }
}

The leak happens because I call a function "startLeak". The activity variable will be leaked. However if I call "startLeak2" the leak won't happen. I am wondering why there is a leak in the first case. The Wrapper uses a WeakReference as well.

The LeakActivity class takes about 30M of memory. Calling startLeak about 5 times on an Android device causes OOM. Calling startLeak2 won't. And LeakCanary tool reports leak if using startLeak not startLeak2.

Check (in the debugger) if the instance of `Callback` has a private field carrying the reference to `activity`. — lexicore, Apr 13 '18 at 20:20
It says there is such a reference from Callback: LeakTracker$1.val$activity. I think probably there is. I don't why the compiler put a reference there. From my human eye there is no need clearly. — darklord, Apr 13 '18 at 20:24
Yes, that's your leak then. Interesting, I've tried in in "normal" Java (I'm not an Android dev) and I don't have any reference to the local variable in the instance of anonymous class. — lexicore, Apr 13 '18 at 20:28
I guess compiler thinks that `Callback` needs the activity and creates the hidden field to reference it. I saw this in "normal" Java, too, when the variable was directly used in the anonymous class. But not in this case. My guess is that Android compiler is not so optimizing compared to "normal" Java. — lexicore, Apr 13 '18 at 20:31

Drilon Blakqori · Answer 1 · 2018-04-13T20:56:04.443

1

On the first method you've declared the activity final. This will add a reference of the activity to the Callback instance, so it's not the Wrapper that is leaking, it's the Callback itself.

Also, keep in mind that Callback is an anonymous inner class and it will also hold a reference to the outer class LeakTracker

edited Apr 13 '18 at 20:56

answered Apr 13 '18 at 20:39

Drilon Blakqori

2,796
2
17
25

But if you just remove "Wrapper wrapper = new Wrapper(activity);" then the reference to final local variable of activity is gone. Looks like that makes the compiler add a hidden reference from Callback to activity. – darklord Apr 13 '18 at 20:42
For this scenario I don't think it has anything to do with the fact that inner class has a hidden reference to outer class – darklord Apr 13 '18 at 20:43
I think it does, if you remove the `Wrapper wrapper = new Wrapper(activity);` then the inner class `Callback` is not using any instance of the activity. Thus it won't have a hidden reference to the activity. Try to remove that line, and inside `oncall()` use the activity instance. This will also make the compiler add a hidden reference of the activity to the `Callback` and thus it will leak just like in your example. – Drilon Blakqori Apr 13 '18 at 20:45
The hidden reference is from Callback not Wrapper. Note Wrapper is static and that Wrapper is not keeping a strong reference to activity. The callback has a hidden strong reference to LeakTracker. But here the leak is for a final local variable activity. Not LeakTracker. – darklord Apr 13 '18 at 20:50
Yea I haven't been clear enough. What I meant was that your `Callback` instance, is making use of the final variable `activity`. This makes it hold a reference to the activity. So your `Wrapper` instance will have a weak reference to the `activity`, but the `Callback` instance will have a strong reference to the `activity`. If you remove `Wrapper wrapper = new Wrapper(activity);` then the `Callback` instance is not making use of the `activity` thus the compiler won't add a reference to it. If you instead use the `activity` reference in the `onCall` method you'll see that it will leak again! – Drilon Blakqori Apr 13 '18 at 20:54
Yeah probably that's what is going on. But can you formalize "using activity variable"? According to @lexicore this hidden reference doesn't exist in a "normal" java environment. And just by judging from the source code there is no need to make such a hidden reference. The only purpose of making a WeakReference to activity is to prevent memory. If the compiler will generates a hidden strong reference anyways, it defeats the purpose of using WeakReference. – darklord Apr 13 '18 at 21:03
1

@darklord The reference isn't within the implementation of `Callback` - if these were normal circumstances (not using an anonymous class), then yes, there would be no strong references. But in this case, that local variable is being *captured*, and although I haven't tested to prove Drilon's theory, I wouldn't doubt this is the cause. – Vince Apr 14 '18 at 01:05

Anonymous class hidden reference to local variable

1 Answers1