1 files changed, 64 insertions, 32 deletions

diff --git a/media/java/android/mtp/MtpDevice.java b/media/java/android/mtp/MtpDevice.java
index e7ea1a5a12cf..d6958b324ca3 100644
--- a/media/java/android/mtp/MtpDevice.java
+++ b/media/java/android/mtp/MtpDevice.java
@@ -25,7 +25,9 @@ import android.os.CancellationSignal;
 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,12 +47,23 @@ public final class MtpDevice {
         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
      *
      * @param device the {@link android.hardware.usb.UsbDevice} for the MTP or PTP device
      */
-    public MtpDevice(UsbDevice device) {
+    public MtpDevice(@NonNull UsbDevice device) {
+        Preconditions.checkNotNull(device);
         mDevice = device;
     }
 
@@ -63,21 +76,29 @@ public final class MtpDevice {
      * @param connection an open {@link android.hardware.usb.UsbDeviceConnection} for the device
      * @return true if the device was successfully opened.
      */
-    public boolean open(UsbDeviceConnection connection) {
+    public boolean open(@NonNull UsbDeviceConnection connection) {
         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();
+            if (!result) {
+                connection.close();
+            } else {
+                mConnection = connection;
+                mCloseGuard.open("close");
+            }
         }
+
         return result;
     }
 
@@ -87,13 +108,23 @@ public final class MtpDevice {
      * 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();
         }
@@ -106,7 +137,7 @@ public final class MtpDevice {
      *
      * @return the device name
      */
-    public String getDeviceName() {
+    public @NonNull String getDeviceName() {
         return mDevice.getDeviceName();
     }
 
@@ -122,16 +153,16 @@ public final class MtpDevice {
     }
 
     @Override
-    public String toString() {
+    public @NonNull String toString() {
         return mDevice.getDeviceName();
     }
 
     /**
      * Returns the {@link MtpDeviceInfo} for this device
      *
-     * @return the device info
+     * @return the device info, or null if fetching device info fails
      */
-    public MtpDeviceInfo getDeviceInfo() {
+    public @Nullable MtpDeviceInfo getDeviceInfo() {
         return native_get_device_info();
     }
 
@@ -139,9 +170,9 @@ public final class MtpDevice {
      * Returns the list of IDs for all storage units on this device
      * Information about each storage unit can be accessed via {@link #getStorageInfo}.
      *
-     * @return the list of storage IDs
+     * @return the list of storage IDs, or null if fetching storage IDs fails
      */
-    public int[] getStorageIds() {
+    public @Nullable int[] getStorageIds() {
         return native_get_storage_ids();
     }
 
@@ -154,9 +185,9 @@ public final class MtpDevice {
      * @param format the format of the object to return, or zero for all formats
      * @param objectHandle the parent object to query, -1 for the storage root,
      *     or zero for all objects
-     * @return the object handles
+     * @return the object handles, or null if fetching object handles fails
      */
-    public int[] getObjectHandles(int storageId, int format, int objectHandle) {
+    public @Nullable int[] getObjectHandles(int storageId, int format, int objectHandle) {
         return native_get_object_handles(storageId, format, objectHandle);
     }
 
@@ -170,7 +201,7 @@ public final class MtpDevice {
      *      {@link MtpObjectInfo#getCompressedSize})
      * @return the object's data, or null if reading fails
      */
-    public byte[] getObject(int objectHandle, int objectSize) {
+    public @Nullable byte[] getObject(int objectHandle, int objectSize) {
         Preconditions.checkArgumentNonnegative(objectSize, "objectSize should not be negative");
         return native_get_object(objectHandle, objectSize);
     }
@@ -188,7 +219,7 @@ public final class MtpDevice {
      * @param buffer Array to write data.
      * @return Size of bytes that are actually read.
      */
-    public long getPartialObject(int objectHandle, long offset, long size, byte[] buffer)
+    public long getPartialObject(int objectHandle, long offset, long size, @NonNull byte[] buffer)
             throws IOException {
         return native_get_partial_object(objectHandle, offset, size, buffer);
     }
@@ -209,7 +240,7 @@ public final class MtpDevice {
      * @return Size of bytes that are actually read.
      * @see MtpConstants#OPERATION_GET_PARTIAL_OBJECT_64
      */
-    public long getPartialObject64(int objectHandle, long offset, long size, byte[] buffer)
+    public long getPartialObject64(int objectHandle, long offset, long size, @NonNull byte[] buffer)
             throws IOException {
         return native_get_partial_object_64(objectHandle, offset, size, buffer);
     }
@@ -224,7 +255,7 @@ public final class MtpDevice {
      * @param objectHandle handle of the object to read
      * @return the object's thumbnail, or null if reading fails
      */
-    public byte[] getThumbnail(int objectHandle) {
+    public @Nullable byte[] getThumbnail(int objectHandle) {
         return native_get_thumbnail(objectHandle);
     }
 
@@ -232,9 +263,9 @@ public final class MtpDevice {
      * Retrieves the {@link MtpStorageInfo} for a storage unit.
      *
      * @param storageId the ID of the storage unit
-     * @return the MtpStorageInfo
+     * @return the MtpStorageInfo, or null if fetching storage info fails
      */
-    public MtpStorageInfo getStorageInfo(int storageId) {
+    public @Nullable MtpStorageInfo getStorageInfo(int storageId) {
         return native_get_storage_info(storageId);
     }
 
@@ -242,9 +273,9 @@ public final class MtpDevice {
      * Retrieves the {@link MtpObjectInfo} for an object.
      *
      * @param objectHandle the handle of the object
-     * @return the MtpObjectInfo
+     * @return the MtpObjectInfo, or null if fetching object info fails
      */
-    public MtpObjectInfo getObjectInfo(int objectHandle) {
+    public @Nullable MtpObjectInfo getObjectInfo(int objectHandle) {
         return native_get_object_info(objectHandle);
     }
 
@@ -291,7 +322,7 @@ public final class MtpDevice {
      *      {@link android.os.Environment#getExternalStorageDirectory}
      * @return true if the file transfer succeeds
      */
-    public boolean importFile(int objectHandle, String destPath) {
+    public boolean importFile(int objectHandle, @NonNull String destPath) {
         return native_import_file(objectHandle, destPath);
     }
 
@@ -305,7 +336,7 @@ public final class MtpDevice {
      * @param descriptor file descriptor to write the data to for the file transfer.
      * @return true if the file transfer succeeds
      */
-    public boolean importFile(int objectHandle, ParcelFileDescriptor descriptor) {
+    public boolean importFile(int objectHandle, @NonNull ParcelFileDescriptor descriptor) {
         return native_import_file(objectHandle, descriptor.getFd());
     }
 
@@ -320,7 +351,8 @@ public final class MtpDevice {
      * @param descriptor file descriptor to read the data from.
      * @return true if the file transfer succeeds
      */
-    public boolean sendObject(int objectHandle, long size, ParcelFileDescriptor descriptor) {
+    public boolean sendObject(
+            int objectHandle, long size, @NonNull ParcelFileDescriptor descriptor) {
         return native_send_object(objectHandle, size, descriptor.getFd());
     }
 
@@ -331,9 +363,9 @@ public final class MtpDevice {
      * The returned {@link MtpObjectInfo} has the new object handle field filled in.
      *
      * @param info metadata of the entry
-     * @return object info of the created entry or null if the operation failed.
+     * @return object info of the created entry, or null if sending object info fails
      */
-    public MtpObjectInfo sendObjectInfo(MtpObjectInfo info) {
+    public @Nullable MtpObjectInfo sendObjectInfo(@NonNull MtpObjectInfo info) {
         return native_send_object_info(info);
     }