Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 03477cb9 authored by Hans Boehm's avatar Hans Boehm
Browse files

Avoid BinderProxy duplicate native registration 

In case of an OOME, we would recycle the nativeData we just allocated,
even if the BinderProxy.getInstance() call got far enough to register
the allocation for automatic freeing. This could cause a duplicate
deallocation.

This changes the code to be much more careful about handling native
deallocation correctly in the exception case.

Bug: 72707270

Test: Build and boot master.
Change-Id: I2cffdd1d59af95f089714893e819c2d02302a6d4
parent 2792c7f6

core/java/android/os/Binder.java

+19 −7
Original line number Diff line number Diff line
@@ -1028,22 +1028,33 @@ final class BinderProxy implements IBinder { 
     * in use, then we return the same bp.

     *

     * @param nativeData C++ pointer to (possibly still empty) BinderProxyNativeData.

     * Takes ownership of nativeData iff <result>.mNativeData == nativeData.  Caller will usually

     * delete nativeData if that's not the case.

     * Takes ownership of nativeData iff <result>.mNativeData == nativeData, or if

     * we exit via an exception.  If neither applies, it's the callers responsibility to

     * recycle nativeData.

     * @param iBinder C++ pointer to IBinder. Does not take ownership of referenced object.

     */

    private static BinderProxy getInstance(long nativeData, long iBinder) {

        BinderProxy result = sProxyMap.get(iBinder);

        if (result == null) {

        BinderProxy result;

        try {

            result = sProxyMap.get(iBinder);

            if (result != null) {

                return result;

            }

            result = new BinderProxy(nativeData);

            sProxyMap.set(iBinder, result);

        } catch (Throwable e) {

            // We're throwing an exception (probably OOME); don't drop nativeData.

            NativeAllocationRegistry.applyFreeFunction(NoImagePreloadHolder.sNativeFinalizer,

                    nativeData);

            throw e;

        }

        NoImagePreloadHolder.sRegistry.registerNativeAllocation(result, nativeData);

        // The registry now owns nativeData, even if registration threw an exception.

        sProxyMap.set(iBinder, result);

        return result;

    }



    private BinderProxy(long nativeData) {

        mNativeData = nativeData;

        NoImagePreloadHolder.sRegistry.registerNativeAllocation(this, mNativeData);

    }



    /**
@@ -1057,8 +1068,9 @@ final class BinderProxy implements IBinder { 
    // Use a Holder to allow static initialization of BinderProxy in the boot image, and

    // to avoid some initialization ordering issues.

    private static class NoImagePreloadHolder {

        public static final long sNativeFinalizer = getNativeFinalizer();

        public static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry(

                BinderProxy.class.getClassLoader(), getNativeFinalizer(), NATIVE_ALLOCATION_SIZE);

                BinderProxy.class.getClassLoader(), sNativeFinalizer, NATIVE_ALLOCATION_SIZE);

    }



    public native boolean pingBinder();

core/jni/android_util_Binder.cpp

+2 −1
Original line number Diff line number Diff line
@@ -663,7 +663,8 @@ jobject javaObjectForIBinder(JNIEnv* env, const sp<IBinder>& val) 
    jobject object = env->CallStaticObjectMethod(gBinderProxyOffsets.mClass,

            gBinderProxyOffsets.mGetInstance, (jlong) nativeData, (jlong) val.get());

    if (env->ExceptionCheck()) {

        gNativeDataCache = nativeData;

        // In the exception case, getInstance still took ownership of nativeData.

        gNativeDataCache = nullptr;

        return NULL;

    }

    BinderProxyNativeData* actualNativeData = getBPNativeData(env, object);