summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSteven Moreland <smoreland@google.com>2021-05-24 19:56:04 +0000
committerSteven Moreland <smoreland@google.com>2021-05-24 19:56:04 +0000
commita926598e896fc3313f545ff6ada7664bea0f3c17 (patch)
tree4a2c31362c4e87f83fc17b64ebe5b8e19dca4104
parent15b49e10ae963a4f6188713866ec4eac32f9b23c (diff)
downloadbase-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.java5
-rw-r--r--core/java/android/os/Parcel.java5
-rw-r--r--core/jni/android_os_Parcel.cpp10
-rw-r--r--core/jni/android_util_Binder.cpp21
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},