Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Reconcile binder JNI with earlier changes, removing lock."

Browse Source
This commit is contained in:
Martijn Coenen
2019-06-07 08:23:00 +00:00
committed by Gerrit Code Review
parent d913216e27 6ccf33b00c
commit c4a564a10a
1 changed files with 17 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
core/jni/android_util_Binder.cpp
View File

@@ -156,9 +156,8 @@ static struct thread_dispatch_offsets_t
static constexpr int32_t PROXY_WARN_INTERVAL = 5000; static constexpr int32_t PROXY_WARN_INTERVAL = 5000;
static constexpr uint32_t GC_INTERVAL = 1000; static constexpr uint32_t GC_INTERVAL = 1000;
// Protected by gProxyLock. We warn if this gets too large. static std::atomic<uint32_t> gNumProxies(0);
static int32_t gNumProxies = 0; static std::atomic<uint32_t> gProxiesWarned(0);
static int32_t gProxiesWarned = 0;
// Number of GlobalRefs held by JavaBBinders. // Number of GlobalRefs held by JavaBBinders.
static std::atomic<uint32_t> gNumLocalRefsCreated(0); static std::atomic<uint32_t> gNumLocalRefsCreated(0);
@@ -660,12 +659,6 @@ BinderProxyNativeData* getBPNativeData(JNIEnv* env, jobject obj) {
return (BinderProxyNativeData *) env->GetLongField(obj, gBinderProxyOffsets.mNativeData); 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. // 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 // 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. // 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; return object;
} }
// For the rest of the function we will hold this lock, to serialize BinderProxyNativeData* nativeData = new BinderProxyNativeData();
// looking/creation/destruction of Java proxies for native Binder proxies. nativeData->mOrgue = new DeathRecipientList;
AutoMutex _l(gProxyLock); nativeData->mObject = val;
BinderProxyNativeData* nativeData = gNativeDataCache;
if (nativeData == nullptr) {
nativeData = new BinderProxyNativeData();
}
// gNativeDataCache is now logically empty.
jobject object = env->CallStaticObjectMethod(gBinderProxyOffsets.mClass, jobject object = env->CallStaticObjectMethod(gBinderProxyOffsets.mClass,
gBinderProxyOffsets.mGetInstance, (jlong) nativeData, (jlong) val.get()); gBinderProxyOffsets.mGetInstance, (jlong) nativeData, (jlong) val.get());
if (env->ExceptionCheck()) { if (env->ExceptionCheck()) {
// In the exception case, getInstance still took ownership of nativeData. // In the exception case, getInstance still took ownership of nativeData.
gNativeDataCache = nullptr;
return NULL; return NULL;
} }
BinderProxyNativeData* actualNativeData = getBPNativeData(env, object); BinderProxyNativeData* actualNativeData = getBPNativeData(env, object);
if (actualNativeData == nativeData) { if (actualNativeData == nativeData) {
// New BinderProxy; we still have exclusive access. // Created a new Proxy
nativeData->mOrgue = new DeathRecipientList; uint32_t numProxies = gNumProxies.fetch_add(1, std::memory_order_relaxed);
nativeData->mObject = val; uint32_t numLastWarned = gProxiesWarned.load(std::memory_order_relaxed);
gNativeDataCache = nullptr; if (numProxies >= numLastWarned + PROXY_WARN_INTERVAL) {
++gNumProxies; // Multiple threads can get here, make sure only one of them gets to
if (gNumProxies >= gProxiesWarned + PROXY_WARN_INTERVAL) { // update the warn counter.
ALOGW("Unexpectedly many live BinderProxies: %d\n", gNumProxies); if (gProxiesWarned.compare_exchange_strong(numLastWarned,
gProxiesWarned = gNumProxies; numLastWarned + PROXY_WARN_INTERVAL, std::memory_order_relaxed)) {
ALOGW("Unexpectedly many live BinderProxies: %d\n", numProxies);
}
} }
} else { } else {
// nativeData wasn't used. Reuse it the next time. delete nativeData;
gNativeDataCache = nativeData;
} }
return object; return object;
@@ -989,8 +977,7 @@ jint android_os_Debug_getLocalObjectCount(JNIEnv* env, jobject clazz)
jint android_os_Debug_getProxyObjectCount(JNIEnv* env, jobject clazz) jint android_os_Debug_getProxyObjectCount(JNIEnv* env, jobject clazz)
{ {
AutoMutex _l(gProxyLock); return gNumProxies.load();
return gNumProxies;
} }
jint android_os_Debug_getDeathObjectCount(JNIEnv* env, jobject clazz) 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) static void BinderProxy_destroy(void* rawNativeData)
{ {
// Don't race with construction/initialization
AutoMutex _l(gProxyLock);
BinderProxyNativeData * nativeData = (BinderProxyNativeData *) rawNativeData; BinderProxyNativeData * nativeData = (BinderProxyNativeData *) rawNativeData;
LOGDEATH("Destroying BinderProxy: binder=%p drl=%p\n", LOGDEATH("Destroying BinderProxy: binder=%p drl=%p\n",
nativeData->mObject.get(), nativeData->mOrgue.get()); nativeData->mObject.get(), nativeData->mOrgue.get());