commit b828b77985416c4d870771a02cd523397726c215
author Philip P. Moltmann <moltmann@google.com> Tue Sep 13 16:35:45 2016 -0700
committer Philip P. Moltmann <moltmann@google.com> Mon Oct 17 11:19:58 2016 -0700
tree 5ff111299700ca511cb2a0648256c83fc20879c3
parent cf6782c9409f98404302eba22c891072291e73b3

Properly close USB device connection in Mtp device

Test: Built
Fixes: 32073045
Change-Id: I05179377532c1bd4dff1f4a4e0e837cb645317e3

media/java/android/mtp/MtpDevice.java

diff --git a/media/java/android/mtp/MtpDevice.java b/media/java/android/mtp/MtpDevice.java
index 4e7551c..d6958b3 100644
--- a/media/java/android/mtp/MtpDevice.java
+++ b/media/java/android/mtp/MtpDevice.java
@@ -25,7 +25,9 @@
 import android.os.ParcelFileDescriptor;
 
 import android.os.UserManager;
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.util.Preconditions;
+import dalvik.system.CloseGuard;
 
 import java.io.IOException;
 
@@ -45,6 +47,16 @@
         System.loadLibrary("media_jni");
     }
 
+    /** Make sure that MTP device is closed properly */
+    @GuardedBy("mLock")
+    private CloseGuard mCloseGuard = CloseGuard.get();
+
+    /** Current connection to the {@link #mDevice}, or null if device is not connected */
+    @GuardedBy("mLock")
+    private UsbDeviceConnection mConnection;
+
+    private final Object mLock = new Object();
+
     /**
      * MtpClient constructor
      *
@@ -68,17 +80,25 @@
         boolean result = false;
 
         Context context = connection.getContext();
-        if (context != null) {
-            UserManager userManager = (UserManager) context.getSystemService(Context.USER_SERVICE);
 
-            if (!userManager.hasUserRestriction(UserManager.DISALLOW_USB_FILE_TRANSFER)) {
-                result = native_open(mDevice.getDeviceName(), connection.getFileDescriptor());
+        synchronized (mLock) {
+            if (context != null) {
+                UserManager userManager = (UserManager) context
+                        .getSystemService(Context.USER_SERVICE);
+
+                if (!userManager.hasUserRestriction(UserManager.DISALLOW_USB_FILE_TRANSFER)) {
+                    result = native_open(mDevice.getDeviceName(), connection.getFileDescriptor());
+                }
+            }
+
+            if (!result) {
+                connection.close();
+            } else {
+                mConnection = connection;
+                mCloseGuard.open("close");
             }
         }
 
-        if (!result) {
-            connection.close();
-        }
         return result;
     }
 
@@ -88,13 +108,23 @@
      * with a new {@link android.hardware.usb.UsbDeviceConnection}.
      */
     public void close() {
-        native_close();
+        synchronized (mLock) {
+            if (mConnection != null) {
+                mCloseGuard.close();
+
+                native_close();
+
+                mConnection.close();
+                mConnection = null;
+            }
+        }
     }
 
     @Override
     protected void finalize() throws Throwable {
         try {
-            native_close();
+            mCloseGuard.warnIfOpen();
+            close();
         } finally {
             super.finalize();
         }

media/jni/android_mtp_MtpDevice.cpp

diff --git a/media/jni/android_mtp_MtpDevice.cpp b/media/jni/android_mtp_MtpDevice.cpp
index 8bcc85f..768ac1d 100644
--- a/media/jni/android_mtp_MtpDevice.cpp
+++ b/media/jni/android_mtp_MtpDevice.cpp
@@ -194,6 +194,9 @@
         return JNI_FALSE;
     }
 
+    // The passed in fd is maintained by the UsbDeviceConnection
+    fd = dup(fd);
+
     MtpDevice* device = MtpDevice::open(deviceNameStr, fd);
     env->ReleaseStringUTFChars(deviceName, deviceNameStr);