Leaking Activity in android.accounts.AccountManager$AmsTask.mActivity

[PromanSEW]

Read this first: https://square.github.io/leakcanary/faq/#can-a-leak-be-caused-by-the-android-sdk

I don't know how to fix this leak so I reported Call AccountManager.addAccount() for leaking Activity

LeakTrace information

│ GC Root: Global variable in native code
│
├─ android.accounts.AccountManager$AmsTask$Response instance
│    Leaking: UNKNOWN
│    Retaining 6,2 MB in 17823 objects
│    ↓ AccountManager$AmsTask$Response.this$1
│                                      ~~~~~~
├─ android.accounts.AccountManager$11 instance
│    Leaking: UNKNOWN
│    Retaining 6,2 MB in 17822 objects
│    Anonymous subclass of android.accounts.AccountManager$AmsTask
│    val$activity instance of com.andromeda.truefishing.ActSettings with
│    mDestroyed = true
│    mActivity instance of com.andromeda.truefishing.ActSettings with
│    mDestroyed = true
│    ↓ AccountManager$AmsTask.mActivity
│                             ~~~~~~~~~
╰→ com.andromeda.truefishing.ActSettings instance
​     Leaking: YES (ObjectWatcher was watching this because com.andromeda.
​     truefishing.ActSettings received Activity#onDestroy() callback and
​     Activity#mDestroyed is true)
​     Retaining 6,2 MB in 17806 objects
​     key = 0b50b034-065c-455d-8dba-09952cd51963
​     watchDurationMillis = 5979
​     retainedDurationMillis = 979
​     mApplication instance of com.andromeda.truefishing.App
​     mBase instance of androidx.appcompat.view.ContextThemeWrapper

METADATA

Build.VERSION.SDK_INT: 31
Build.MANUFACTURER: samsung
LeakCanary version: 2.12
App process name: com.andromeda.truefishing.rustore
Class count: 27115
Instance count: 247156
Primitive array count: 160422
Object array count: 34569
Thread count: 49
Heap total bytes: 29806580
Bitmap count: 576
Bitmap total bytes: 55853704
Large bitmap count: 0
Large bitmap total bytes: 0
Db 1: closed /[data/user/0/com.andromeda.truefishing](http://data/user/0/com.andromeda.truefishing).
rustore/databases/google_app_measurement_local.db
Db 2: open /[data/user/0/com.andromeda.truefishing.rustore/databases/firestore](http://data/user/0/com.andromeda.truefishing.rustore/databases/firestore).
%5BDEFAULT%5D.api-project-849953414391.%28default%29
Db 3: open /[data/user/0/com.andromeda.truefishing.rustore/databases/com.google](http://data/user/0/com.andromeda.truefishing.rustore/databases/com.google).
[android.datatransport.events](http://android.datatransport.events/)
Stats: LruCache[maxSize=3000,hits=123952,misses=228070,hitRate=35%]
RandomAccess[bytes=11465361,reads=228070,travel=108704621727,range=36869891,size
=44967864]
Analysis duration: 14129 ms

[PromanSEW]

Source of leak:

return new AmsTask(activity, handler, callback) {
   @Override
    public void doWork() throws RemoteException {
       mService.addAccount(mResponse, accountType, authTokenType,
          requiredFeatures, activity != null, optionsIn);
    }
}.start();

[radityagumay]

looks like it's sort of classic lapsed listener problem

@PromanSEW is that possible to add stop in the appropriate callback from the host UI controller? or maybe you can wrap the activity as a WeakReference

[PromanSEW]

@radityagumay AmsTask is part of Android SDK, so WeakReference is not a solution. How I can fix leak if I have this instance of AmsTask?

private abstract class AmsTask extends FutureTask<Bundle> implements AccountManagerFuture<Bundle> {
...
    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
    final Activity mActivity;
...
}

Where links: FutureTask, AccountManagerFuture

[pyricau]

The leak trace shows AccountManager$11. The 11th anonymous class from the top in AccountManager on Android 12 is defined here: https://cs.android.com/android/platform/superproject/+/android-12.0.0_r34:frameworks/base/core/java/android/accounts/AccountManager.java;l=1957-1962;drc=a74b54f7f913511b52d20ee15fe10b8a99966397

    public AccountManagerFuture<Bundle> addAccount(final String accountType,
            final String authTokenType, final String[] requiredFeatures,
            final Bundle addAccountOptions,
            final Activity activity, AccountManagerCallback<Bundle> callback, Handler handler) {
        if (Process.myUserHandle().equals(mContext.getUser())) {
            if (accountType == null) throw new IllegalArgumentException("accountType is null");
            final Bundle optionsIn = new Bundle();
            if (addAccountOptions != null) {
                optionsIn.putAll(addAccountOptions);
            }
            optionsIn.putString(KEY_ANDROID_PACKAGE_NAME, mContext.getPackageName());

            return new AmsTask(activity, handler, callback) {
                @Override
                public void doWork() throws RemoteException {
                    mService.addAccount(mResponse, accountType, authTokenType,
                            requiredFeatures, activity != null, optionsIn);
                }
            }.start();
        } else {
            return addAccountAsUser(accountType, authTokenType, requiredFeatures, addAccountOptions,
                    activity, callback, handler, mContext.getUser());
        }
    }

Zooming in:

            return new AmsTask(activity, handler, callback) {
                @Override
                public void doWork() throws RemoteException {
                    mService.addAccount(mResponse, accountType, authTokenType,
                            requiredFeatures, activity != null, optionsIn);
                }
            }.start();

That part of the leak trace also had:

├─ android.accounts.AccountManager$11 instance
│    Leaking: UNKNOWN
│    Retaining 6,2 MB in 17822 objects
│    Anonymous subclass of android.accounts.AccountManager$AmsTask
│    val$activity instance of com.andromeda.truefishing.ActSettings with
│    mDestroyed = true
│    mActivity instance of com.andromeda.truefishing.ActSettings with
│    mDestroyed = true
│    ↓ AccountManager$AmsTask.mActivity
│                             ~~~~~~~~~

=> The AccountManager$11 we found in the sources is definitely an anonymous subclass of AccountManager$AmsTask.

The top of the leak trace shows:

│ GC Root: Global variable in native code
│
├─ android.accounts.AccountManager$AmsTask$Response instance

That class is defined here:

https://cs.android.com/android/platform/superproject/+/android-12.0.0_r34:frameworks/base/core/java/android/accounts/AccountManager.java;l=2439;drc=a74b54f7f913511b52d20ee15fe10b8a99966397

        private class Response extends IAccountManagerResponse.Stub {
            @Override
            public void onResult(Bundle bundle) {

So this is a classic IPC stub leak. Implementations of stubs should never hold final strong references to objects with lifecycle (services, activities), because the nature of stubs mean they will be in memory even after we stop needing them, until the other process has had a GC run.

Note to self: I wonder if we should add labels that immediately call out when we run into a Stub into a leak trace as it's always the same type of leak caused by bugs in the Android Framework.

@PromanSEW you (or @radityagumay) should file a ticket to the Android Framework.

[pyricau]

Ping, did you file a ticket to the Android framework?

[pyricau]

Note: this was filed once before, and closed with no update: https://issuetracker.google.com/issues/37046509

Worth opening a new one.

[pyricau]

Filed https://issuetracker.google.com/issues/318303120

[pyricau]

aaaand google just closed it again with "won't fix, obsolete". WTF.

[radityagumay]

aaaand google just closed it again with "won't fix, obsolete". WTF.