tree 02051a6f18e37fe137f57ed5d2c4780615c3e7c3
parent 9fb33563c2e0e7271361b208ec3d620cad74c6c7
author Jiyong Park <jiyong@google.com> 1694161055 +0900
committer Jiyong Park <jiyong@google.com> 1695649905 +0900

binder: fix death recipient leak for apps targeting >= V

Before this change, when a death recipient is set on a binder proxy via
linkToDeath, a JNI global ref to the recipient object was created. That
global ref is cleared only when unlinkToDeath is explicitly called or
binderDied is notified.

In addition, since binderDied didn't give the IBinder which has died,
people has kept a strong reference to IBinder in the death recipient
object. Ex:

class FooHolder implements Binder.DeathRecipient {
    private IFoo mFoo;
    public FooHolder(IFoo foo) {
        mFoo = foo; // this!!!
        mFoo.linkToDeath(this, 0);
    }
    @Override
    public void binderDied() {
        // know that IFoo has died
    }
}

Unfortunately, this is prone to leak. Even if there's no reference to
FooHolder in your program, it is kept in memory due to the JNI global
ref as mentioned above. It means that you keep IFoo as well, and that
in turn keeps the binder service in the remote side. As a result,
binderDied will never be called (well, except when the server process
crashes).

The only way to release this object is calling unlinkToDeath explicitly
when you drop references to FooHolder. However, it's error prone and
keeping that practice is hard to be enforced.

Recently, the need for this pattern has become weaker as we introduced
binderDied(IBinder who). However, the API is quite new and its use is
not mandated. There still are many cases where this pattern is used.

This change is an attempt to fix the issue without having to touch the
existing uses. The idea is to change the way that death recipient
objects are strongly referenced - depending on whether you are targeting
Android V+ or not.

If targeting Android V+, the death recipient object is "weakly"
referenced from JNI. Instead, it is "strongly" referenced from the
BinderProxy object it is registered at. This means that if you drop
a BinderProxy object, you are dropping its death recipients as well,
unless you keep references to the recipients separately.

For apps targeting pre-V versions, we keep the JNI strong reference.

An important implication of this is that you won't get binderDied if you
drop BinderProxy object before the binder actually dies. This actually
is the documented behavior and has been the actual behavior "if you
don't use the FooHolder pattern mentioned above". I'd argue that this CL
fixes the undocumented incorrect behavior. However, we should be
conservative when making any behavioral change, thus we are hiding this
change behind the target SDK level.

Bug: 298374304
Test: atest BinderLeakTest BinderLeakTest_legacy

Change-Id: Ibb371f4de45530670d5f783f8ead8404c39381b4