Image adapter leaking memory

Question

I have a simple ListActivity that shows images and I inizialize my OkHttpClient for Picasso Builder in the constructor of the ImageAdapter class:

picassoClient = new OkHttpClient();
picassoClient.interceptors().add(new Interceptor() {
    @Override
    public Response intercept(Chain chain) throws IOException {
        Request newRequest = chain
            .request()
            .newBuilder()
            .addHeader("Cookie","xyz")
            .build();

        return chain.proceed(newRequest);
    }
});

new Picasso.Builder(context).downloader(new OkHttpDownloader(picassoClient)).build();

then in getView() I use Picasso to load images in ImageView:

Picasso.with(context).load(xyzUrl).fit().centerCrop().into(vImage);

It works well, but on device's rotation i see that heap size sometimes slowly grows, sometimes quickly and sometimes remains stable. Only rarely it drops. Am i leaking memory or is there something wrong in code?

EDIT: I inserted this code after Picasso's call in the getView()

if (BuildConfig.DEBUG) {
    Log.i("HEAP SIZE",
    String.valueOf((Runtime.getRuntime().totalMemory() / 1024)
    - (Runtime.getRuntime().freeMemory() / 1024)));
}

and I found that the heap size's growth happens in the getView() after loading bitmap into ImageView. What is wrong?

EDIT 2: tried to set static ImageAdapter, nothing changes

EDIT 3: tried with RecyclerView instead of ListView, same behavior: heap size grows continuously while scrolling image list stepping by 30-40 bytes at every onBindViewHolder(). After device's rotation heap size grows sometimes stepping by even 2-3 Mbytes. Rarely it drops.

Why heap size slowly but continuously grows and why am I leaking some cache or some cached bitmaps after device's rotation?

UPDATE: tried adapter without the code in the constructor (that is without new OkHttpClient and new Picasso.Builder), it works and the heap size now drops well remaining stable. Then, what is the correct way to initialize the client with cookies headers management?

UPSHOT: finally I created my PicassoInstance class, which creates a unique static Picasso singleton and set it as the Picasso Library's singleton. Then I set it in my adapter constructor

PicassoInstance.setPicassoSingleton(context);

It works well, and it is a correct way I hope.

public class PicassoInstance {
private static Picasso myPicassoInstance = null;

public static void setPicassoSingleton(Context context) {
    if (myPicassoInstance == null) {
        myPicassoInstance = createMyPicassoInstance(context);
        Picasso.setSingletonInstance(myPicassoInstance);
        if (BuildConfig.DEBUG) {
            Log.i("PICASSO INSTANCE", "CREATED");
        }
    }
}

private static Picasso createMyPicassoInstance(Context context) {
    OkHttpClient myOkHttpClient = new OkHttpClient();
    myOkHttpClient.interceptors().add(new Interceptor() {
        @Override
        public Response intercept(Chain chain) throws IOException {
            Request newRequest = chain.request().newBuilder()
                    .addHeader("Cookie", "xyz").build();
            if (BuildConfig.DEBUG) {
                Log.i("ON INTERCEPT", "COOKIE ADDED");
            }
            return chain.proceed(newRequest);
        }
    });

    return new Picasso.Builder(context).downloader(
            new OkHttpDownloader(myOkHttpClient)).build();
}

}

only `setListAdapter(adapter);` in `onCreate()`, nothing static, no other life cycle method. — GPack, Apr 24 '15 at 21:19
Then I'm guessing that the adapter is leaking something. I you explicitly null it in onPause(), any change? if not, might be worth showing your adapter code. Do you bind it to anything else? — Simon, Apr 24 '15 at 21:38
I edited my question, I found that the heap size's growth happens in the getView(). — GPack, Apr 25 '15 at 14:50
Picasso caches images in memory for quick retrieval so my guess is it's just keeping references to the images until it feels the need to remove them. I wouldn't worry about it until you produce a OOM error. — DeeV, Apr 26 '15 at 15:25
In fact it works well, however if I change the orientation dozens of times I get an OOM. — GPack, Apr 26 '15 at 15:32
From picasso docs `Use with(android.content.Context) for the global singleton instance or construct your own instance with Picasso.Builder.`. So after you make the OkHttpClient, OkHttpDownloader and custom picasso instance...you throw it away. Or was that for illustration? — Eugen Pechanec, Apr 27 '15 at 15:20
is this the same as [picasso-issue#968: PicassoDrawable's placeholder is not released if the drawable becomes another PicassoDrawable's placeholder.](https://github.com/square/picasso/issues/968) ? — k3b, Apr 27 '15 at 15:22
@Eugen Pechanec I need to add cookie headers to get the images. — GPack, Apr 27 '15 at 15:25
I understand what you need to do, but you're not doing it. You construct the picasso instance and that's all. You don't store it, nobody uses it, it gets destroyed after the method ends. The images are loaded by `Picasso.with(context)` which is the default unmodified singleton instance. I suggest you create a `MyPicasso` singleton class for your custom picasso client. — Eugen Pechanec, Apr 27 '15 at 15:30
tried to use `mPicasso=new Picasso.Builder(context)...` : on rotation heap size returns to grow. — GPack, Apr 27 '15 at 16:16
The easiest way to track memory allocations is to use android's MAT utility. It gives a list of class objects that are not freed from the memory. You can read about its usage here: http://android-developers.blogspot.in/2011/03/memory-analysis-for-android.html — Gagan, Apr 29 '15 at 07:09
fyi you only ever want 1 `OkHttpClient` so `private static OkHttpClient myClient;` then in method: `if(myClient == null) { myClient = new OkHttpClient()` otherwise how else can it manage cache etc ... — Blundell, May 02 '15 at 23:16

dabluck · Accepted Answer · 2015-05-03T00:09:38.400

7

The picasso instance being returned from PicassoBuilder.build() should be a singleton, and when you need to use picasso throughout the app you should be accessing that singleton, instead of Picasso.with... you should be accessing

YourClass.getMyPicassoSingleton().with...

Otherwise you're keeping separate caches, etc for those picasso instances

edit: as I noted below, you can also call

picasso.setSingletonInstance(myPicasso);

right where you invoke the build method above, which would also solve your problem without holding onto the singleton yourself. that is probably a cleaner solution

edited May 03 '15 at 00:09

answered May 02 '15 at 21:09

dabluck

1,641
2
13
20

2

i think under the hood picasso also does this and returns you a "singleton" each time anyway – Blundell May 02 '15 at 23:14
1

https://github.com/square/picasso/blob/master/picasso/src/main/java/com/squareup/picasso/Picasso.java if you invoke the builder yourself, it does not set its internal singleton. so if you invoke the builder method and build your own, and then go ahead and use Picasso.with later, you've created two instances. just look at the code it's very straightforward. note that you can set the singleton on picasso with setsingletoninstance which would be an equally valid answer for OP and would solve the problem as well – dabluck May 02 '15 at 23:18
1

glad you commented so i actually looked at the class and saw an easier solution haha – dabluck May 02 '15 at 23:25
well, one question please: `Picasso.setSingletonInstance(myPicasso)` must be called once then how can I check this behavior after device's rotation, must be my own instance _myPicasso_ static too? – GPack May 03 '15 at 08:15
i'm not clear what you mean. you only call setsingletoninstance once, like when you create the okhttp client. then on rotation you should just be using the picasso.with call and resetting the imageview. does that make sense? – dabluck May 03 '15 at 22:33

score 3 · Answer 2 · edited May 23 '17 at 12:25

I cannot close it as too broad, but I'd recommend you took a memory dump and gave it a long hard look in Eclipse Memory Analizer Tool to find which references are still being kept alive and by who.

This is also a great write up on it.

Adapters as fields are leaky. Views contain context which contains views. And fragments are even worse offenders. ListActivities were an API1 tool that should have been deprecated long ago. All very leak prone, but that's the Android way.

Image adapter leaking memory

2 Answers2