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

Commit 6ccf33b0 authored by Martijn Coenen's avatar Martijn Coenen
Browse files

Reconcile binder JNI with earlier changes, removing lock. 

Change Idbacb6ad14c43aff8030d70b5e17427b86e92d6e introduced
synchronization of the sProxyMap in Java, removing the need
for serialization (and having a native lock). But for some
reason, we did not remove it in AOSP.

Bug: 132922399
Test: builds
Change-Id: Ib2dc64a7716a1e4798b998e54fac298eb40a7c1d
Merged-In: I4e06b3f93e30ed1c7868ec9e018709a7e796e441
parent 2bba8e7b

core/jni/android_util_Binder.cpp

+17 −33
Original line number Diff line number Diff line
@@ -156,9 +156,8 @@ static struct thread_dispatch_offsets_t 
static constexpr int32_t PROXY_WARN_INTERVAL = 5000;

static constexpr uint32_t GC_INTERVAL = 1000;



// Protected by gProxyLock. We warn if this gets too large.

static int32_t gNumProxies = 0;

static int32_t gProxiesWarned = 0;

static std::atomic<uint32_t> gNumProxies(0);

static std::atomic<uint32_t> gProxiesWarned(0);



// Number of GlobalRefs held by JavaBBinders.

static std::atomic<uint32_t> gNumLocalRefsCreated(0);
@@ -660,12 +659,6 @@ BinderProxyNativeData* getBPNativeData(JNIEnv* env, jobject obj) { 
    return (BinderProxyNativeData *) env->GetLongField(obj, gBinderProxyOffsets.mNativeData);

}



static Mutex gProxyLock;



// We may cache a single BinderProxyNativeData node to avoid repeat allocation.

// All fields are null. Protected by gProxyLock.

static BinderProxyNativeData *gNativeDataCache;



// If the argument is a JavaBBinder, return the Java object that was used to create it.

// Otherwise return a BinderProxy for the IBinder. If a previous call was passed the

// same IBinder, and the original BinderProxy is still alive, return the same BinderProxy.
@@ -680,36 +673,31 @@ jobject javaObjectForIBinder(JNIEnv* env, const sp<IBinder>& val) 
        return object;

    }



    // For the rest of the function we will hold this lock, to serialize

    // looking/creation/destruction of Java proxies for native Binder proxies.

    AutoMutex _l(gProxyLock);

    BinderProxyNativeData* nativeData = new BinderProxyNativeData();

    nativeData->mOrgue = new DeathRecipientList;

    nativeData->mObject = val;



    BinderProxyNativeData* nativeData = gNativeDataCache;

    if (nativeData == nullptr) {

        nativeData = new BinderProxyNativeData();

    }

    // gNativeDataCache is now logically empty.

    jobject object = env->CallStaticObjectMethod(gBinderProxyOffsets.mClass,

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

    if (env->ExceptionCheck()) {

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

        gNativeDataCache = nullptr;

        return NULL;

    }

    BinderProxyNativeData* actualNativeData = getBPNativeData(env, object);

    if (actualNativeData == nativeData) {

        // New BinderProxy; we still have exclusive access.

        nativeData->mOrgue = new DeathRecipientList;

        nativeData->mObject = val;

        gNativeDataCache = nullptr;

        ++gNumProxies;

        if (gNumProxies >= gProxiesWarned + PROXY_WARN_INTERVAL) {

            ALOGW("Unexpectedly many live BinderProxies: %d\n", gNumProxies);

            gProxiesWarned = gNumProxies;

        // Created a new Proxy

        uint32_t numProxies = gNumProxies.fetch_add(1, std::memory_order_relaxed);

        uint32_t numLastWarned = gProxiesWarned.load(std::memory_order_relaxed);

        if (numProxies >= numLastWarned + PROXY_WARN_INTERVAL) {

            // Multiple threads can get here, make sure only one of them gets to

            // update the warn counter.

            if (gProxiesWarned.compare_exchange_strong(numLastWarned,

                        numLastWarned + PROXY_WARN_INTERVAL, std::memory_order_relaxed)) {

                ALOGW("Unexpectedly many live BinderProxies: %d\n", numProxies);

            }

        }

    } else {

        // nativeData wasn't used. Reuse it the next time.

        gNativeDataCache = nativeData;

        delete nativeData;

    }



    return object;
@@ -989,8 +977,7 @@ jint android_os_Debug_getLocalObjectCount(JNIEnv* env, jobject clazz) 


jint android_os_Debug_getProxyObjectCount(JNIEnv* env, jobject clazz)

{

    AutoMutex _l(gProxyLock);

    return gNumProxies;

    return gNumProxies.load();

}



jint android_os_Debug_getDeathObjectCount(JNIEnv* env, jobject clazz)
@@ -1385,9 +1372,6 @@ static jboolean android_os_BinderProxy_unlinkToDeath(JNIEnv* env, jobject obj, 


static void BinderProxy_destroy(void* rawNativeData)

{

    // Don't race with construction/initialization

    AutoMutex _l(gProxyLock);



    BinderProxyNativeData * nativeData = (BinderProxyNativeData *) rawNativeData;

    LOGDEATH("Destroying BinderProxy: binder=%p drl=%p\n",

            nativeData->mObject.get(), nativeData->mOrgue.get());