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

Commit 0f888800 authored by Jayant Chowdhary's avatar Jayant Chowdhary
Browse files

Close MQDescriptor internal FDs while writing to Parcel to transfer to native code. 



Though ParcelFileDescriptors do close the wrapped fds in finalize(), GC
may not run for a while so it is beneficial to close them explicitly.

Also recycle the temporary Parcel written into by MQDescriptor since
that dups the MQDescriptor's fds as well. They're not needed after the
native FMQ Reader has been created since the MQDescriptor in the native
code dups those descriptors upon creation from the AParcel as well.

Bug: 383702975

Flag: EXEMPT; bugfix

Test: Run test app with StrictMode on, no longer get android.os.strictmode.LeakedClosableViolation
      for ParceFileDescriptor

Change-Id: Ic2a49577f1e0da13dbc33ef14a2a437966aebc99
Signed-off-by: default avatarJayant Chowdhary <jchowdhary@google.com>
parent 9165f171

core/java/android/hardware/camera2/impl/CameraDeviceImpl.java

+12 −1
Original line number Diff line number Diff line
@@ -482,8 +482,19 @@ public class CameraDeviceImpl extends CameraDevice 


            mRemoteDevice = new ICameraDeviceUserWrapper(remoteDevice);

            Parcel resultParcel = Parcel.obtain();

            mRemoteDevice.getCaptureResultMetadataQueue().writeToParcel(resultParcel, 0);



            // Passing in PARCELABLE_WRITE_RETURN_VALUE closes the ParcelFileDescriptors

            // owned by MQDescriptor returned by getCaptureResultMetadataQueue()

            // Though these will be closed when GC runs, that may not happen for a while.

            // Also, apps running with StrictMode would get warnings / crash in the case they're not

            // explicitly closed.

            mRemoteDevice.getCaptureResultMetadataQueue().writeToParcel(resultParcel,

                    Parcelable.PARCELABLE_WRITE_RETURN_VALUE);

            mFMQReader = nativeCreateFMQReader(resultParcel);

            // Recycle since resultParcel would dup fds from MQDescriptor as well. We don't

            // need them after the native FMQ reader has been created. That is since the native

            // creates calls MQDescriptor.readFromParcel() which again dups the fds.

            resultParcel.recycle();



            IBinder remoteDeviceBinder = remoteDevice.asBinder();

            // For legacy camera device, remoteDevice is in the same process, and

core/jni/android_hardware_camera2_CameraDevice.cpp

+7 −4
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ 
#include <nativehelper/JNIHelp.h>

#include "android_os_Parcel.h"

#include "core_jni_helpers.h"

#include <android/binder_auto_utils.h>

#include <android/binder_parcel_jni.h>

#include <android/hardware/camera2/ICameraDeviceUser.h>

#include <aidl/android/hardware/common/fmq/MQDescriptor.h>
@@ -40,6 +41,7 @@ 
using namespace android;



using ::android::AidlMessageQueue;

using ndk::ScopedAParcel;

using ResultMetadataQueue = AidlMessageQueue<int8_t, SynchronizedReadWrite>;



class FMQReader {
@@ -75,15 +77,16 @@ extern "C" { 


static jlong CameraDevice_createFMQReader(JNIEnv *env, jclass thiz,

        jobject resultParcel) {

    AParcel *resultAParcel = AParcel_fromJavaParcel(env, resultParcel);

    if (resultAParcel == nullptr) {

    ScopedAParcel sResultAParcel(AParcel_fromJavaParcel(env, resultParcel));

    if (sResultAParcel.get() == nullptr) {

        ALOGE("%s: Error creating result parcel", __FUNCTION__);

        return 0;

    }

    AParcel_setDataPosition(resultAParcel, 0);



    AParcel_setDataPosition(sResultAParcel.get(), 0);



    MQDescriptor<int8_t, SynchronizedReadWrite> resultMQ;

    if (resultMQ.readFromParcel(resultAParcel) != OK) {

    if (resultMQ.readFromParcel(sResultAParcel.get()) != OK) {

        ALOGE("%s: read from result parcel failed", __FUNCTION__);

        return 0;

    }