diff options
author | Steven Moreland <smoreland@google.com> | 2021-05-24 19:56:04 +0000 |
---|---|---|
committer | Steven Moreland <smoreland@google.com> | 2021-05-24 19:56:04 +0000 |
commit | a926598e896fc3313f545ff6ada7664bea0f3c17 (patch) | |
tree | 4a2c31362c4e87f83fc17b64ebe5b8e19dca4104 | |
parent | 15b49e10ae963a4f6188713866ec4eac32f9b23c (diff) | |
download | base-a926598e896fc3313f545ff6ada7664bea0f3c17.tar.gz |
Revert "binder: race condition by parcel finalize"
Revert "binder: race condition by parcel finalize"
Revert submission 1553275-Bug139327211_V5
Reason for revert: causing a different race condition? b/187218964
Reverted Changes:
I9345f4439:binder: race condition by parcel finalize
Ib06e38e22:binder: race condition by parcel finalize
Change-Id: Iebf76febdad5e231c814f1f8de454931cb9352c5
-rw-r--r-- | core/java/android/os/BinderProxy.java | 5 | ||||
-rw-r--r-- | core/java/android/os/Parcel.java | 5 | ||||
-rw-r--r-- | core/jni/android_os_Parcel.cpp | 10 | ||||
-rw-r--r-- | core/jni/android_util_Binder.cpp | 21 |
4 files changed, 8 insertions, 33 deletions
diff --git a/core/java/android/os/BinderProxy.java b/core/java/android/os/BinderProxy.java index 483838bbe49a..683993f762c0 100644 --- a/core/java/android/os/BinderProxy.java +++ b/core/java/android/os/BinderProxy.java @@ -537,8 +537,7 @@ public final class BinderProxy implements IBinder { } try { - boolean replyOwnsNative = (reply == null) ? false : reply.ownsNativeParcelObject(); - return transactNative(code, data, reply, replyOwnsNative, flags); + return transactNative(code, data, reply, flags); } finally { AppOpsManager.resumeNotedAppOpsCollection(prevCollection); @@ -563,7 +562,7 @@ public final class BinderProxy implements IBinder { * Native implementation of transact() for proxies */ public native boolean transactNative(int code, Parcel data, Parcel reply, - boolean replyOwnsNativeParcelObject, int flags) throws RemoteException; + int flags) throws RemoteException; /** * See {@link IBinder#linkToDeath(DeathRecipient, int)} */ diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index 248f7ae90058..cf90174924f1 100644 --- a/core/java/android/os/Parcel.java +++ b/core/java/android/os/Parcel.java @@ -3690,9 +3690,4 @@ public final class Parcel { public long getBlobAshmemSize() { return nativeGetBlobAshmemSize(mNativePtr); } - - /** @hide */ - /*package*/ boolean ownsNativeParcelObject() { - return mOwnsNativeParcelObject; - } } diff --git a/core/jni/android_os_Parcel.cpp b/core/jni/android_os_Parcel.cpp index 0fdab722dd05..241570a7f9d3 100644 --- a/core/jni/android_os_Parcel.cpp +++ b/core/jni/android_os_Parcel.cpp @@ -36,7 +36,6 @@ #include <utils/List.h> #include <utils/KeyedVector.h> #include <binder/Parcel.h> -#include <binder/ParcelRef.h> #include <binder/ProcessState.h> #include <binder/IServiceManager.h> #include <utils/threads.h> @@ -530,9 +529,8 @@ static jobject android_os_Parcel_readFileDescriptor(JNIEnv* env, jclass clazz, j static jlong android_os_Parcel_create(JNIEnv* env, jclass clazz) { - sp<ParcelRef> parcelRef = ParcelRef::create(); - parcelRef->incStrong(reinterpret_cast<const void*>(android_os_Parcel_create)); - return reinterpret_cast<jlong>(static_cast<Parcel *>(parcelRef.get())); + Parcel* parcel = new Parcel(); + return reinterpret_cast<jlong>(parcel); } static jlong android_os_Parcel_freeBuffer(JNIEnv* env, jclass clazz, jlong nativePtr) @@ -547,8 +545,8 @@ static jlong android_os_Parcel_freeBuffer(JNIEnv* env, jclass clazz, jlong nativ static void android_os_Parcel_destroy(JNIEnv* env, jclass clazz, jlong nativePtr) { - ParcelRef* derivative = static_cast<ParcelRef*>(reinterpret_cast<Parcel*>(nativePtr)); - derivative->decStrong(reinterpret_cast<const void*>(android_os_Parcel_create)); + Parcel* parcel = reinterpret_cast<Parcel*>(nativePtr); + delete parcel; } static jbyteArray android_os_Parcel_marshall(JNIEnv* env, jclass clazz, jlong nativePtr) diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp index f71b42c4793f..581dc0848a28 100644 --- a/core/jni/android_util_Binder.cpp +++ b/core/jni/android_util_Binder.cpp @@ -35,7 +35,6 @@ #include <binder/IPCThreadState.h> #include <binder/IServiceManager.h> #include <binder/Parcel.h> -#include <binder/ParcelRef.h> #include <binder/ProcessState.h> #include <binder/Stability.h> #include <binderthreadstate/CallerUtils.h> @@ -1375,8 +1374,7 @@ static bool should_time_binder_calls() { } static jboolean android_os_BinderProxy_transact(JNIEnv* env, jobject obj, - jint code, jobject dataObj, jobject replyObj, jboolean replyObjOwnsNativeParcel, - jint flags) // throws RemoteException + jint code, jobject dataObj, jobject replyObj, jint flags) // throws RemoteException { if (dataObj == NULL) { jniThrowNullPointerException(env, NULL); @@ -1418,21 +1416,6 @@ static jboolean android_os_BinderProxy_transact(JNIEnv* env, jobject obj, status_t err = target->transact(code, *data, reply, flags); //if (reply) printf("Transact from Java code to %p received: ", target); reply->print(); - if (reply) { - if (replyObjOwnsNativeParcel) { - // as per Parcel java class constructor, here, "reply" MUST be a "ParcelRef" - // only for Parcel that contained Binder objects - if (reply->objectsCount() > 0) { - IPCThreadState::self()->createTransactionReference(static_cast<ParcelRef*>(reply)); - } - } else { - // as per Parcel.java, if Parcel java object NOT owning native Parcel object, it will - // NOT destroy the native Parcel object upon GC(finalize()), so, there will be no race - // condtion in this case. Please refer to the java class methods: Parcel.finalize(), - // Parcel.destroy(). - } - } - if (kEnableBinderSample) { if (time_binder_calls) { conditionally_log_binder_call(start_millis, target, code); @@ -1559,7 +1542,7 @@ static const JNINativeMethod gBinderProxyMethods[] = { {"pingBinder", "()Z", (void*)android_os_BinderProxy_pingBinder}, {"isBinderAlive", "()Z", (void*)android_os_BinderProxy_isBinderAlive}, {"getInterfaceDescriptor", "()Ljava/lang/String;", (void*)android_os_BinderProxy_getInterfaceDescriptor}, - {"transactNative", "(ILandroid/os/Parcel;Landroid/os/Parcel;ZI)Z", (void*)android_os_BinderProxy_transact}, + {"transactNative", "(ILandroid/os/Parcel;Landroid/os/Parcel;I)Z", (void*)android_os_BinderProxy_transact}, {"linkToDeath", "(Landroid/os/IBinder$DeathRecipient;I)V", (void*)android_os_BinderProxy_linkToDeath}, {"unlinkToDeath", "(Landroid/os/IBinder$DeathRecipient;I)Z", (void*)android_os_BinderProxy_unlinkToDeath}, {"getNativeFinalizer", "()J", (void*)android_os_BinderProxy_getNativeFinalizer}, |