DEV Community

Py βš”
Py βš”

Posted on • Edited on

Beware PackageManager leaks!

While I was investigating an Android memory leak, I took these notes as I learnt about the lifecycle of android.app.ContextImpl and android.app.ApplicationPackageManager. I started poking at cs.android.com and somehow ended up decompiling the Android 11 framework bytecode πŸ˜…. If you don't know what ContextImpl is, read on!

Take away: if you need to keep a PackagerManager instance, get it from the application Context.

// Don't do this:
HelperThing.packageManager = activity.packageManager

// Do this instead:
HelperThing.packageManager = activity.applicationContext.packageManager
Enter fullscreen mode Exit fullscreen mode

A Wild Leak Appears!

At Square, I configured our debug builds to upload leak traces to Bugsnag. That way, we can find which leaks happen the most and investigate them.

Leak reports on Bugsnag

I started looking into a leak that didn't have anything to do with our app's code:

┬───
β”‚ GC Root: System class
β”‚
β”œβ”€ android.app.ApplicationPackageManager class
β”‚    Leaking: NO (a class is never leaking)
β”‚    ↓ static ApplicationPackageManager.mHasSystemFeatureCache
β”‚                                       ~~~~~~~~~~~~~~~~~~~~~~
β”œβ”€ android.app.ApplicationPackageManager$1 instance
β”‚    Leaking: UNKNOWN
β”‚    Anonymous subclass of android.app.PropertyInvalidatedCache
β”‚    ↓ ApplicationPackageManager$1.mCache
β”‚                                  ~~~~~~
β”œβ”€ android.app.PropertyInvalidatedCache$1 instance
β”‚    Leaking: UNKNOWN
β”‚    Anonymous subclass of java.util.LinkedHashMap
β”‚    ↓ PropertyInvalidatedCache$1.tail
β”‚                                 ~~~~
β”œβ”€ java.util.LinkedHashMap$LinkedHashMapEntry instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ LinkedHashMap$LinkedHashMapEntry.key
β”‚                                       ~~~
β”œβ”€ android.app.ApplicationPackageManager$HasSystemFeatureQuery instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ ApplicationPackageManager$HasSystemFeatureQuery.this$0
β”‚                                                      ~~~~~~
β”œβ”€ android.app.ApplicationPackageManager instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ ApplicationPackageManager.mContext
β”‚                                ~~~~~~~~
β”œβ”€ android.app.ContextImpl instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ ContextImpl.mAutofillClient
β”‚                  ~~~~~~~~~~~~~~~
β•°β†’ com.example.MainActivity instance
     Leaking: YES (ObjectWatcher was watching this because 
     com.example.MainActivity received Activity#onDestroy() callback and
     Activity#mDestroyed is true)
Enter fullscreen mode Exit fullscreen mode

The name ApplicationPackageManager seem to imply this package manager is an application scoped object, ie a singleton. Therefore it's not leaking, and the leak is probably somewhere after ApplicationPackageManager instance.

...
β”œβ”€ android.app.ApplicationPackageManager instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ ApplicationPackageManager.mContext
β”‚                                ~~~~~~~~
β”œβ”€ android.app.ContextImpl instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ ContextImpl.mAutofillClient
β”‚                  ~~~~~~~~~~~~~~~
β•°β†’ com.example.MainActivity instance
     Leaking: YES
Enter fullscreen mode Exit fullscreen mode

ContextImpl.mAutofillClient seems fairly suspicious, let's look into it!

A primer on Context & ContextWrapper

On Android, Context (source) is a god object that provides access to app resources. Major Android components (Application, Activity, Service) extend Context. More specifically, they extend ContextWrapper (source), a subclass of Context that delegates to a base context.

The Application class hierarchy looks like this:

/**
 * Interface to global information about an application environment.
 */
abstract class Context 
  abstract fun getFilesDir(): File

  // More methods to access app resources...
}

/**
 * Proxying implementation of Context that simply delegates all of its calls to
 * another Context.
 */
open class ContextWrapper : Context {
  private var mBase: Context? = null

  protected fun attachBaseContext(base: Context) {
    mBase = base
  }

  override fun getFilesDir() = mBase.getFilesDir()
}

/**
 * Base class for maintaining global application state.
 */
open class Application : ContextWrapper() {
  fun onCreate() = Unit
}
Enter fullscreen mode Exit fullscreen mode

Where is the actual implementation of Context.getFilesDir()? In ContextImpl!

/**
 * Common implementation of Context API, which provides the base
 * context object for Activity and other application components.
 */
class ContextImpl : Context {
  override fun getFilesDir(): File {
    synchronized(mSync) {
      if (mFilesDir == null) {
        mFilesDir = File(dataDir, "files")
      }
      return ensurePrivateDirExists(mFilesDir)
    }
  }
}
Enter fullscreen mode Exit fullscreen mode

Back to ContextImpl.mAutofillClient

Activity overrides attachBaseContext() to set the autofill client of its base context to itself (source):

open class Activity : ContextThemeWrapper() {
  override fun attachBaseContext(newBase: Context) {
    super.attachBaseContext(newBase)
    if (newBase != null) {
      newBase.autofillClient = this
    }
  }
}
Enter fullscreen mode Exit fullscreen mode

Since the base context of an activity is ContextImpl, this part of the leak trace now makes sense:

...
β”œβ”€ android.app.ContextImpl instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ ContextImpl.mAutofillClient
β”‚                  ~~~~~~~~~~~~~~~
β•°β†’ com.example.MainActivity instance
Enter fullscreen mode Exit fullscreen mode

When is mAutofillClient set back to null? Never (source).

Search results on cs.android.com

So, is that what's causing this leak? Should the activity clear the autofill client on its base context when it gets destroyed?

Before jumping to conclusions, we need to understand the lifecycle of ContextImpl. Is it an application scoped singleton? Is it activity scoped? Something else?

The lifecycle of ContextImpl

It turns out, every Android component that is a Context has a ContextImpl set as its base context. ContextImpl.outerContext points back to the Android component it serves.

Calls to ContextImpl.setOuterContext

This teaches us that the lifecycle of ContextImpl is tied to the lifecycle of its outer context.

We can now update the leak trace:

...
β”œβ”€ android.app.ApplicationPackageManager instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ ApplicationPackageManager.mContext
β”‚                                ~~~~~~~~
β”œβ”€ android.app.ContextImpl instance
β”‚    Leaking: YES (ContextImpl.mOuterContext is an instance of 
|    com.example.MainActivity with Activity.mDestroyed true)
β”‚    ↓ ContextImpl.mAutofillClient
β”‚                  ~~~~~~~~~~~~~~~
β•°β†’ com.example.MainActivity instance
     Leaking: YES (ObjectWatcher was watching this because 
     com.example.MainActivity received Activity#onDestroy() callback and
     Activity#mDestroyed is true)
Enter fullscreen mode Exit fullscreen mode

What about ApplicationPackageManager then?

Is ApplicationPackageManager application scoped?

ApplicationPackagerManager instances are created in ContextImpl.getPackageManager() (source):

class ContextImpl : Context {
  override fun getPackageManager(): PackageManager {
    if (mPackageManager != null) {
      return mPackageManager
    }
    IPackageManager pm = ActivityThread.getPackageManager()
    mPackageManager = ApplicationPackageManager(this, pm)
    return mPackageManager
  }
}
Enter fullscreen mode Exit fullscreen mode

This teaches us that the lifecycle of ApplicationPackagerManager is tied to the lifecycle of the ContextImpl that created it, which is tied to the lifecycle of its outer context.

My early assumption that ApplicationPackagerManager is an app wide singleton was incorrect!

Take away: if you need to keep a PackagerManager instance, get it from the application Context.

// Don't do this:
HelperThing.packageManager = activity.packageManager

// Do this instead:
HelperThing.packageManager = activity.applicationContext.packageManager
Enter fullscreen mode Exit fullscreen mode

I updated LeakCanary to take this new information into consideration. We can update the leak trace once again:

┬───
β”‚ GC Root: System class
β”‚
β”œβ”€ android.app.ApplicationPackageManager class
β”‚    Leaking: NO (a class is never leaking)
β”‚    ↓ static ApplicationPackageManager.mHasSystemFeatureCache
β”‚                                       ~~~~~~~~~~~~~~~~~~~~~~
β”œβ”€ android.app.ApplicationPackageManager$1 instance
β”‚    Leaking: UNKNOWN
β”‚    Anonymous subclass of android.app.PropertyInvalidatedCache
β”‚    ↓ ApplicationPackageManager$1.mCache
β”‚                                  ~~~~~~
β”œβ”€ android.app.PropertyInvalidatedCache$1 instance
β”‚    Leaking: UNKNOWN
β”‚    Anonymous subclass of java.util.LinkedHashMap
β”‚    ↓ PropertyInvalidatedCache$1.tail
β”‚                                 ~~~~
β”œβ”€ java.util.LinkedHashMap$LinkedHashMapEntry instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ LinkedHashMap$LinkedHashMapEntry.key
β”‚                                       ~~~
β”œβ”€ android.app.ApplicationPackageManager$HasSystemFeatureQuery instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ ApplicationPackageManager$HasSystemFeatureQuery.this$0
β”‚                                                      ~~~~~~
β”œβ”€ android.app.ApplicationPackageManager instance
β”‚    Leaking: YES (ApplicationContextManager.mContext.mOuterContext is an
|    instance of com.example.MainActivity with
|    Activity.mDestroyed true)
β”‚    ↓ ApplicationPackageManager.mContext
β”‚                                ~~~~~~~~
β”œβ”€ android.app.ContextImpl instance
β”‚    Leaking: YES (ContextImpl.mOuterContext is an instance of 
|    com.example.MainActivity with Activity.mDestroyed true)
β”‚    ↓ ContextImpl.mAutofillClient
β”‚                  ~~~~~~~~~~~~~~~
β•°β†’ com.example.MainActivity instance
     Leaking: YES (ObjectWatcher was watching this because 
     com.example.MainActivity received Activity#onDestroy() callback and
     Activity#mDestroyed is true)
Enter fullscreen mode Exit fullscreen mode

πŸ€”

Let's zoom in on the beginning of the leak trace:

┬───
β”‚ GC Root: System class
β”‚
β”œβ”€ android.app.ApplicationPackageManager class
β”‚    Leaking: NO (a class is never leaking)
β”‚    ↓ static ApplicationPackageManager.mHasSystemFeatureCache
β”‚                                       ~~~~~~~~~~~~~~~~~~~~~~
β”œβ”€ android.app.ApplicationPackageManager$1 instance
...
Enter fullscreen mode Exit fullscreen mode

Something's weird: ApplicationPackageManager has a static field named mHasSystemFeatureCache, which is an LRU cache. According to the AOSP field naming conventions:

  • Non-public, non-static field names start with m.
  • Static field names start with s.

We can also see that ApplicationPackageManager.HasSystemFeatureQuery is an inner non static class which holds on to its outer class, the ApplicationPackageManager instance (via this$0), but it's also a key in the ApplicationPackageManager.mHasSystemFeatureCache static LRU cache.

...
β”œβ”€ android.app.ApplicationPackageManager$HasSystemFeatureQuery instance
β”‚    Leaking: UNKNOWN
β”‚    ↓ ApplicationPackageManager$HasSystemFeatureQuery.this$0
β”‚                                                      ~~~~~~
...
Enter fullscreen mode Exit fullscreen mode

All of this looks suspicious, we should take a closer look, but we cannot find ApplicationPackageManager.mHasSystemFeatureCache in the Android sources.

At Square, we've had this leak on a Pixel 3 XL (Android 11 DP 2). On StackOverflow someone reported it on a Pixel 4 XL (Android 11 DP 2), there's also a bug report on the emulator (Android 11 DP) and another on the Pixel 4 XL (Android 11 DP 2).

So, this looks like an Android 11 leak in developer previews. Unfortunately, the sources aren't available yet. Or are they?

Decompiling framework.jar on Android 11

Lucky for us, Google provides us with Android 11 emulator images. We can start an emulator and pull the Android framework bytecode:

adb pull /system/framework/framework.jar
Enter fullscreen mode Exit fullscreen mode

framework.jar contains dex files:

META-INF
android
classes.dex
classes2.dex
classes3.dex
classes4.dex
res
Enter fullscreen mode Exit fullscreen mode

We can decompile those dex files with dex2jar. classes2.dex contains the ApplicationPackageManager class:

public class ApplicationPackageManager extends PackageManager {
  private static final PropertyInvalidatedCache<HasSystemFeatureQuery, Boolean> 
    mHasSystemFeatureCache = new PropertyInvalidatedCache<HasSystemFeatureQuery,
      Boolean>(256, "cache_key.has_system_feature") {
    protected Boolean recompute(HasSystemFeatureQuery var1) { ... }
  };

  private static final class HasSystemFeatureQuery {
    public final String name;
    public final int version;

    public HasSystemFeatureQuery(String var1, int var2) { ... }

    public boolean equals(Object var1) { ... }

    public int hashCode() { ... }

    public String toString() { ... }
  }

}
Enter fullscreen mode Exit fullscreen mode

We found the ApplicationPackageManager.mHasSystemFeatureCache static field and the ApplicationPackageManager.HasSystemFeatureQuery inner static class. In the leak trace, ApplicationPackageManager.HasSystemFeatureQuery was an inner non static class, but now we can see it's a static class so it looks like this leak was fixed in the latest Android 11 release.

Conclusion

Edit: we have the sources now! The leak was introduced here and fixed here, a month later. Unfortunately Android 11 DP 2 was already cut.

Wow, that was quite a ride. I had no idea this blog would be so long when I started investigating. I hope your learnt as much as I did and that you enjoyed it. Let me know on Twitter!

Top comments (2)

Collapse
 
samlu profile image
Sam Lu

I found the ActivityManager has the same issue when we get the ActivityManager instance via an Activity context and keep it with a static variable.

No leakage if we get the ActivityManager instance via the Application context. I.e.
ctx.getApplicationContext().getSystemService(Context.ACTIVITY_SERVICE)

Collapse
 
dimezis profile image
Dmitry Saviuk

Every time something in Android asks me for the Context, I use application one :)
Always a safer bet