Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Android memory leak in Apache Harmony's JarURLConnectionImpl?

I'm working on an Android app and we're investigating memory use.

Looking at a heap dump from hprof, we're seeing nearly 2M (22% of our heap) being used in a static cache in JarURLConnectionImpl:

enter image description here

Looking at the source code for JarURLConnectionImpl, it appears that entries are added to the static jarCache variable, but never removed.

If it's true that they're never removed, that strikes me as a potential memory leak.

Is this a leak? Is there a fix or workaround?

like image 569
emmby Avatar asked Jan 30 '13 17:01

emmby


2 Answers

Here's an ugly workaround:

private static HashMap<URL,JarFile> jarCache;


static {
    try {
        Class<?> jarURLConnectionImplClass = Class.forName("org.apache.harmony.luni.internal.net.www.protocol.jar.JarURLConnectionImpl");
        final Field jarCacheField = jarURLConnectionImplClass.getDeclaredField("jarCache");
        jarCacheField.setAccessible(true);
        //noinspection unchecked
        jarCache = (HashMap<URL, JarFile>) jarCacheField.get(null);
    } catch(Exception e) {
        // ignored
    }
}

Then, periodically run the following:

    // HACK http://stackoverflow.com/questions/14610350/android-memory-leak-in-apache-harmonys-jarurlconnectionimpl
    if( jarCache!=null ) {
        try {
            for (
                final Iterator<Map.Entry<URL, JarFile>> iterator = jarCache.entrySet().iterator(); iterator.hasNext(); ) {
                final Map.Entry<URL, JarFile> e = iterator.next();
                final URL url = e.getKey();
                if (Strings.toString(url).endsWith(".apk")) {
                    Log.i(TAG,"Removing static hashmap entry for " + url);
                    try {
                        final JarFile jarFile = e.getValue();
                        jarFile.close();
                        iterator.remove();
                    } catch( Exception f ) {
                        Log.e(TAG,"Error removing hashmap entry for "+ url,f);
                    }
                }
            }
        } catch( Exception e ) {
            // ignored
        }
    }

I run it on activity creation so it gets executed every time one of my activities is created. The ugly hashmap entry doesn't seem to get recreated all that often, but it DOES seem to reappear occasionally, so it's not sufficient to just run this code once.

like image 155
emmby Avatar answered Nov 15 '22 17:11

emmby


This is definitely a nasty memory leak. I've opened an issue for it since no one else seems to have reported it.

Thanks for the "ugly workaround" emmby, that was helpful. A safer approach, although potentially with a performance impact, is to disable URLConnection caching altogether. Since the URLConnection.defaultUseCaches flag is static and, as you might guess, is the default for each instance's useCaches flag, you can just set this to false and no more instances will cache their connections. This will affect all implementations of URLConnection, so it may have farther-ranging effects than desired, but I think it's a reasonable trade-off.

You can just create a simple class like this and instantiate it very early in your app's onCreate():

public class URLConnectionNoCache extends URLConnection {

    protected URLConnectionNoCache(URL url) {
        super(url);
        setDefaultUseCaches(false);
    }

    public void connect() throws IOException {
    }
}

The interesting thing is that since this occurs after your app is loaded and run, the system libs should already be cached, and this will only prevent further caching, so this probably gives the best possible trade-off: not caching your apk while allowing the performance benefits of caching the system jars.

Before doing this I did modify emmby's solution a bit to make it a standalone class that creates a background thread to periodically clear the cache. And I restricted it to just clear the app's apk, though that can be relaxed if desired. The thing to worry about here is that you're modifying the objects while they may be in use, which is generally not a good thing. If you do want to go this route you just need to call the start() method with a context, e.g. in your app's onCreate().

package com.example;

import java.lang.reflect.Field;
import java.net.URL;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.jar.JarFile;
import java.util.regex.Pattern;

import android.content.Context;

// hack to remove memory leak in JarURLConnectionImpl
// from http://stackoverflow.com/questions/14610350/android-memory-leak-in-apache-harmonys-jarurlconnectionimpl

public class JarURLMonitor {
    private static JarURLMonitor instance;
    private Pattern pat;
    private Field jarCacheField;
    public volatile boolean stop;

    private static final long CHECK_INTERVAL = 60 * 1000;


    public static synchronized void start(Context context) {
        if (instance == null) {
            instance = new JarURLMonitor(context);
        }
    }

    public static synchronized void stop() {
        if (instance != null) {
            instance.stop = true;
        }
    }

    private JarURLMonitor(Context context) {
        // get jar cache field
        try {
            final Class<?> cls = Class.forName("libcore.net.url.JarURLConnectionImpl");
            jarCacheField = cls.getDeclaredField("jarCache");
            jarCacheField.setAccessible(true);
        }
        catch (Exception e) {
            // log
        }

        if (jarCacheField != null) {
            // create pattern that matches our package: e.g. /data/app/<pkgname>-1.apk
            pat = Pattern.compile("^.*/" + context.getPackageName() + "-.*\\.apk$");

            // start background thread to check it
            new Thread("JarURLMonitor") {
                @Override
                public void run() {
                    try {
                        while (!stop) {
                            checkJarCache();
                            Thread.sleep(CHECK_INTERVAL);
                        }
                    }
                    catch (Exception e) {
                        // log
                    }
                }
            }.start();
        }
    }

    private void checkJarCache() throws Exception {
        @SuppressWarnings("unchecked")
        final HashMap<URL, JarFile> jarCache = (HashMap<URL, JarFile>)jarCacheField.get(null);

        final Iterator<Map.Entry<URL, JarFile>> iterator = jarCache.entrySet().iterator();
        while (iterator.hasNext()) {
            final Map.Entry<URL, JarFile> entry = iterator.next();
            final JarFile jarFile = entry.getValue();
            final String file = jarFile.getName();
            if (pat.matcher(file).matches()) {
                try {
                    jarFile.close();
                    iterator.remove();
                }
                catch (Exception e) {
                    // log
                }
            }
        }
    }
}
like image 25
Peter Jeffe Avatar answered Nov 15 '22 17:11

Peter Jeffe