Watch destroyed services

[ChaosLeung]

This may help detect leaks of services.

Before API 19, ActivityThread.mServices is a HashMap. The ActivityThread.handleStopService method will remove the service instance stored in mServices, and then Service.onDestroy method will be called. So, we can set a new HashMap and override the remove method to listen Service.onDestroy. (Yes, inspired by #2007)

Start from API 19, ActivityThread.mServices is an ArrayMap, and ArrayMap is a final class. We only can use ActivityThread.H.STOP_SERVICE to listen Service.onDestroy.

Test on API 16:

Test on API 29:

[CLAassistant]

All committers have signed the CLA.

[ChaosLeung]

On a second thought, this approach is imperfect, both of them happen before onDestroy.

[pyricau]

This is really cool! Is this hitting any API grey list / black list?

[ChaosLeung]

Yes, all of them are in greylist.

From the android-11-lists:

Landroid/app/ActivityManager;->IActivityManagerSingleton:Landroid/util/Singleton;
Landroid/app/ActivityThread;->currentActivityThread()Landroid/app/ActivityThread;
Landroid/app/ActivityThread;->mServices:Landroid/util/ArrayMap;
Landroid/app/ActivityThread;->mH:Landroid/app/ActivityThread$H;
Landroid/os/Handler;->mCallback:Landroid/os/Handler$Callback;
Landroid/util/Singleton;->mInstance:Ljava/lang/Object;

But WindowManagerGlobal is also described in greylist.

Landroid/view/WindowManagerGlobal;->getInstance()Landroid/view/WindowManagerGlobal;
Landroid/view/WindowManagerGlobal;->mViews:Ljava/util/ArrayList;

Should I provide a config to disable the watcher like leak_canary_watcher_install_root_view_detach?

[pyricau]

Should I provide a config to disable the watcher like leak_canary_watcher_install_root_view_detach?

Depends on whether this has a chance of breaking apps. The RootView hack installs a different array list instance so quite risky.

[ChaosLeung]

Okay, the destroyed service hack has same risk.

[ChaosLeung]

The new approach:

• PreDestroyed: Set a different Handler.Callback object to ActivityThread.mH.mCallback for tracing ActivityThread.H.STOP_SERVICE message. Every time the Handler.Callback receive the message, we can get the pending destroy Service's token (msg.obj) and instance (from ActivityThread.mServices).

• Destroyed: After the Service instance was called onDestroy, ActivityThread will call IActivityManager#serviceDoneExecuting (call flow of ActivityThread#handleStopService). So we can set a dynamic proxy to ActivityManager.IActivityManagerSingleton (start from API 26) or ActivityManagerNative.gDefault (before API 26) for tracing IActivityManager#serviceDoneExecuting. When receive a call of serviceDoneExecuting, we can get the Service's token (fist parameter of serviceDoneExecuting) to find out matching Service instance and watch it.

Due to the risk in this approach, now the ServiceDestroyWatcher can be disabled via:

<?xml version="1.0" encoding="utf-8"?>
<resources>
  <bool name="leak_canary_watcher_install_service_destroy">false</bool>
</resources>

[pyricau]

Thanks! Sorry for the slow review, this is really cool but I need time to dig into it

[pyricau]

Going through the code on cs.android.com

Another thing that's called is ActivityThread.removeContextRegistrations() which removes an entry from ActivityThread.receivers and ActivityThread.unregisteredReceivers which are ArrayMap instances (interestingly only receivers has @UnsupportedAppUsage. Not sure if we can leverage that, just wanted to write it down. ArrayMap is a final class so it looks like we can't hook in listeners there.

[pyricau]

We likely want a mutableMapOf<IBinder, WeakReference> to not end up being the cause of a service leak, and maybe even a WeakHashMap<IBinder, WeakReference> (ie weak key as well because idk if we should risk retaining the IBinder either)

[pyricau]

Followed up here: #2029

[pyricau]

yeah ok this is cool. Replace the handler callback and delegate to it, which ensures that this stays compatible with any other code running the same hack.

[pyricau]

Note: merged main to fix conflicts.

[ChaosLeung]

Another thing that's called is ActivityThread.removeContextRegistrations() which removes an entry from ActivityThread.receivers and ActivityThread.unregisteredReceivers which are ArrayMap instances (interestingly only receivers has @UnsupportedAppUsage. Not sure if we can leverage that, just wanted to write it down. ArrayMap is a final class so it looks like we can't hook in listeners there.

We can handle the ActivityThread.H.CLEAN_UP_CONTEXT message. When receiving the message, we can get all leaking receivers from LoadedApk.mReceivers. But this approach only work for dynamic BroadcastReceiver but not static (Maybe we don’t need to care about static BroadcastReceiver).

[pyricau]

Does this mean we could use this to detect leaking receivers? I'm not sure what defines a leak for a receiver exactly. After deregistration?

[ChaosLeung]

I'm not sure what defines a leak for a receiver exactly. After deregistration?

In my opinion, a leaked receiver means that its associated context has been destroyed (such as destroyed Activity/Service) and it has been unregistered.

Does this mean we could use this to detect leaking receivers?

In source of ActivityThread, the ActivityThread.H.CLEAN_UP_CONTEXT message only sent in handleDestroyActivity, handleStopService. Therefore, the approach mentioned above can only detect those receivers registered via Activity or Service are leaked or not.

On second thought, it is difficult to cover all scenarios. Because there is no way to detect those receivers registered with application context are leaked or not.