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

Commit 674e8c38 authored by Philip P. Moltmann's avatar Philip P. Moltmann
Browse files

Do not leak usb device connection FD 

The native code uses mNativeContext=0 to indicate that is was already
closed and checks this properly. Hence let's leave the checking in the
native code.

We need to keep a reference in UsbRequest as otherwise the
UsbDeviceConnection might get finalized while a UsbRequest is in
progress. The UsbRequest itself makes sure that it is not garbage
collected while the I/O is in progress.

Also I added CloseGuards to make sure the classes are used properly and
fixed an error string in the native code.

Fixes: 31124312
Bug: 31021315
Change-Id: I96deb73957eba0e14e6b656988a2ae9b409bf55f
parent 79758c8e

core/java/android/hardware/usb/UsbDeviceConnection.java

+24 −2
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package android.hardware.usb; 


import android.annotation.SystemApi;

import android.os.ParcelFileDescriptor;

import dalvik.system.CloseGuard;



import java.io.FileDescriptor;
@@ -35,6 +36,8 @@ public class UsbDeviceConnection { 
    // used by the JNI code

    private long mNativeContext;



    private final CloseGuard mCloseGuard = CloseGuard.get();



    /**

     * UsbDevice should only be instantiated by UsbService implementation

     * @hide
@@ -44,7 +47,13 @@ public class UsbDeviceConnection { 
    }



    /* package */ boolean open(String name, ParcelFileDescriptor pfd) {

        return native_open(name, pfd.getFileDescriptor());

        boolean wasOpened = native_open(name, pfd.getFileDescriptor());



        if (wasOpened) {

            mCloseGuard.open("close");

        }



        return wasOpened;

    }



    /**
@@ -54,7 +63,10 @@ public class UsbDeviceConnection { 
     * to retrieve a new instance to reestablish communication with the device.

     */

    public void close() {

        if (mNativeContext != 0) {

            native_close();

            mCloseGuard.close();

        }

    }



    /**
@@ -262,6 +274,16 @@ public class UsbDeviceConnection { 
        }

    }



    @Override

    protected void finalize() throws Throwable {

        try {

            mCloseGuard.warnIfOpen();

            close();

        } finally {

            super.finalize();

        }

    }



    private native boolean native_open(String deviceName, FileDescriptor pfd);

    private native void native_close();

    private native int native_get_fd();

core/java/android/hardware/usb/UsbRequest.java

+24 −8
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package android.hardware.usb;



import android.util.Log;

import dalvik.system.CloseGuard;



import java.nio.ByteBuffer;
@@ -48,6 +49,11 @@ public class UsbRequest { 
    // for client use

    private Object mClientData;



    // Prevent the connection from being finalized

    private UsbDeviceConnection mConnection;



    private final CloseGuard mCloseGuard = CloseGuard.get();



    public UsbRequest() {

    }
@@ -60,25 +66,35 @@ public class UsbRequest { 
     */

    public boolean initialize(UsbDeviceConnection connection, UsbEndpoint endpoint) {

        mEndpoint = endpoint;

        return native_init(connection, endpoint.getAddress(), endpoint.getAttributes(),

                endpoint.getMaxPacketSize(), endpoint.getInterval());

        mConnection = connection;



        boolean wasInitialized = native_init(connection, endpoint.getAddress(),

                endpoint.getAttributes(), endpoint.getMaxPacketSize(), endpoint.getInterval());



        if (wasInitialized) {

            mCloseGuard.open("close");

        }



        return wasInitialized;

    }



    /**

     * Releases all resources related to this request.

     */

    public void close() {

        if (mNativeContext != 0) {

            mEndpoint = null;

            mConnection = null;

            native_close();

            mCloseGuard.close();

        }

    }



    @Override

    protected void finalize() throws Throwable {

        try {

            if (mEndpoint != null) {

                Log.v(TAG, "endpoint still open in finalize(): " + this);

            mCloseGuard.warnIfOpen();

            close();

            }

        } finally {

            super.finalize();

        }

core/jni/android_hardware_UsbDeviceConnection.cpp

+1 −1
Original line number Diff line number Diff line
@@ -235,7 +235,7 @@ android_hardware_UsbDeviceConnection_get_serial(JNIEnv *env, jobject thiz) 
{

    struct usb_device* device = get_device_from_object(env, thiz);

    if (!device) {

        ALOGE("device is closed in native_request_wait");

        ALOGE("device is closed in native_get_serial");

        return NULL;

    }

    char* serial = usb_device_get_serial(device);