From 60bf84a129fe742ac2737527336069c487f285f0 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Wed, 8 Feb 2017 10:22:28 +0100 Subject: [PATCH] Don't generate and send reply object in oneway calls. Bug: 35044790 Test: hidl_test, hidl_test_java Change-Id: Iae8f5b071f89d2af5ca15360c6a1a0fbf8040e77 --- core/jni/android_os_HwBinder.cpp | 47 ++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/core/jni/android_os_HwBinder.cpp b/core/jni/android_os_HwBinder.cpp index d35ffbba3a1e1..1f7efa1ae9efb 100644 --- a/core/jni/android_os_HwBinder.cpp +++ b/core/jni/android_os_HwBinder.cpp @@ -127,18 +127,23 @@ status_t JHwBinder::onTransact( uint32_t flags, TransactCallback callback) { JNIEnv *env = AndroidRuntime::getJNIEnv(); + bool isOneway = (flags & TF_ONE_WAY) != 0; + ScopedLocalRef replyObj(env, nullptr); + sp replyContext = nullptr; ScopedLocalRef requestObj(env, JHwParcel::NewObject(env)); JHwParcel::GetNativeContext(env, requestObj.get())->setParcel( const_cast(&data), false /* assumeOwnership */); - ScopedLocalRef replyObj(env, JHwParcel::NewObject(env)); - sp replyContext = - JHwParcel::GetNativeContext(env, replyObj.get()); + if (!isOneway) { + replyObj.reset(JHwParcel::NewObject(env)); - replyContext->setParcel(reply, false /* assumeOwnership */); - replyContext->setTransactCallback(callback); + replyContext = JHwParcel::GetNativeContext(env, replyObj.get()); + + replyContext->setParcel(reply, false /* assumeOwnership */); + replyContext->setTransactCallback(callback); + } env->CallVoidMethod( mObject, @@ -166,27 +171,29 @@ status_t JHwBinder::onTransact( status_t err = OK; - if (!replyContext->wasSent()) { - // The implementation never finished the transaction. - err = UNKNOWN_ERROR; // XXX special error code instead? + if (!isOneway) { + if (!replyContext->wasSent()) { + // The implementation never finished the transaction. + err = UNKNOWN_ERROR; // XXX special error code instead? + + reply->setDataPosition(0 /* pos */); + } + + // Release all temporary storage now that scatter-gather data + // has been consolidated, either by calling the TransactCallback, + // if wasSent() == true or clearing the reply parcel (setDataOffset above). + replyContext->getStorage()->release(env); + + // We cannot permanently pass ownership of "data" and "reply" over to their + // Java object wrappers (we don't own them ourselves). + replyContext->setParcel( + NULL /* parcel */, false /* assumeOwnership */); - reply->setDataPosition(0 /* pos */); } - // Release all temporary storage now that scatter-gather data - // has been consolidated, either by calling the TransactCallback, - // if wasSent() == true or clearing the reply parcel (setDataOffset above). - replyContext->getStorage()->release(env); - - // We cannot permanently pass ownership of "data" and "reply" over to their - // Java object wrappers (we don't own them ourselves). - JHwParcel::GetNativeContext(env, requestObj.get())->setParcel( NULL /* parcel */, false /* assumeOwnership */); - replyContext->setParcel( - NULL /* parcel */, false /* assumeOwnership */); - return err; }