Beware PackageManager leaks!
Tags: android, aosp, leak, leakcanary
Published: 2020-06-29 19:00:29

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](https://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`.

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

// Do this instead:
HelperThing.packageManager = activity.applicationContext.packageManager
```

# A Wild Leak Appears!

At Square, I configured our debug builds to [upload leak traces to Bugsnag](https://square.github.io/leakcanary/recipes/#uploading-to-bugsnag). That way, we can find which leaks happen the most and investigate them.

![Leak reports on Bugsnag](https://dev-to-uploads.s3.amazonaws.com/i/3nbyvqqqd6c9gl9uvwc8.png)

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)
```

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
```

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

# A primer on Context & ContextWrapper

{% twitter 1277637604024840194 %}

On Android, `Context` ([source](https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/content/Context.java;drc=master;l=99)) is a god object that provides access to app resources. Major Android components (`Application`, `Activity`, `Service`) extend `Context`. More specifically, they extend `ContextWrapper` ([source](https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/content/ContextWrapper.java;l=60;drc=master)), a subclass of `Context` that delegates to a base context.

The `Application` class hierarchy looks like this:

```kotlin
/**
 * 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
}
```

Where is the actual implementation of `Context.getFilesDir()`? In [ContextImpl](https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/app/ContextImpl.java;l=174;drc=master)!

```kotlin
/**
 * 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)
    }
  }
}
```

# Back to `ContextImpl.mAutofillClient`

`Activity` overrides `attachBaseContext()` to set the autofill client of its base context to itself ([source](https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/app/Activity.java;l=1122-1128;drc=master;bpv=1;bpt=1)):

```kotlin
open class Activity : ContextThemeWrapper() {
  override fun attachBaseContext(newBase: Context) {
    super.attachBaseContext(newBase)
    if (newBase != null) {
      newBase.autofillClient = this
    }
  }
}
```

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
```

When is `mAutofillClient` set back to `null`? Never ([source](https://cs.android.com/search?q=setAutofillClient&sq=&ss=android)).

![Search results on cs.android.com](https://dev-to-uploads.s3.amazonaws.com/i/zngs880sgexj0rxhmlj1.png)

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](https://dev-to-uploads.s3.amazonaws.com/i/wmz7l3lgr0im5f2mh9te.png)

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)
```

What about `ApplicationPackageManager` then?

# Is ApplicationPackageManager application scoped?

`ApplicationPackagerManager` instances are created in `ContextImpl.getPackageManager()` ([source](https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/app/ContextImpl.java;l=302-315?q=ContextImpl&ss=android)):

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

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`.

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

// Do this instead:
HelperThing.packageManager = activity.applicationContext.packageManager
```

I [updated LeakCanary](https://github.com/square/leakcanary/pull/1884/commits/107c7921f32813cc433b7905dd385cdcd8fb4fa7) 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)
```

# 🤔

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
...
```

Something's weird: `ApplicationPackageManager` has a `static` field named `mHasSystemFeatureCache`, which is an LRU cache. According to the AOSP [field naming conventions](https://source.android.com/setup/contribute/code-style#follow-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
│                                                      ~~~~~~
...
```

All of this looks suspicious, we should take a closer look, but we cannot find `ApplicationPackageManager.mHasSystemFeatureCache` in the [Android sources](https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/app/ApplicationPackageManager.java).

At Square, we've had this leak on a Pixel 3 XL (**Android 11 DP 2**). On StackOverflow someone [reported it](https://stackoverflow.com/questions/60849541/could-this-leak-be-caused-by-the-android-11-dp2) on a Pixel 4 XL (**Android 11 DP 2**), there's also a [bug report](https://issuetracker.google.com/issues/153260804) on the **emulator** (Android 11 DP) and [another](https://issuetracker.google.com/issues/152549757) 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
```

`framework.jar` contains dex files:

```
META-INF
android
classes.dex
classes2.dex
classes3.dex
classes4.dex
res
```

We can decompile those dex files with [dex2jar](https://github.com/pxb1988/dex2jar). `classes2.dex` contains the `ApplicationPackageManager` class:

```java
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() { ... }
  }

}
```

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](https://cs.android.com/android/_/android/platform/frameworks/base/+/89608118192580ffca026b5dacafa637a556d578) and fixed [here](https://cs.android.com/android/_/android/platform/frameworks/base/+/1f771846c51148b7cb6283e6dc82a216ffaa5353), 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](https://twitter.com/Piwai)!