commit 7063c936e50326883e672837036381f77080c2f3
author Jerry Zhang <zhangjerry@google.com> Tue Apr 04 15:06:10 2017 -0700
committer Jerry Zhang <zhangjerry@google.com> Thu Apr 06 12:05:40 2017 -0700
tree 2c64eedc087ee2bdd5de64368bb73ed87c040a7b
parent 97e6e199f9470cea3d686e0ed1033b171747356a

Wait for threads to finish before returning.

Prevents the thread struct from being deallocated
before the thread has finished.

Bug: 35152606
Test: Transfer files in both directions, verify behavior when
interrupted

Change-Id: Ic247072234977709711366636e6a39031fbc125a

media/mtp/AsyncIO.cpp

diff --git a/media/mtp/AsyncIO.cpp b/media/mtp/AsyncIO.cpp
index e77ad38..bfb07dc 100644
--- a/media/mtp/AsyncIO.cpp
+++ b/media/mtp/AsyncIO.cpp
@@ -96,6 +96,10 @@
 
 } // end anonymous namespace
 
+aiocb::~aiocb() {
+    CHECK(!thread.joinable());
+}
+
 void aio_pool_init(void(f)(int)) {
     CHECK(done == 1);
     done = 0;

media/mtp/AsyncIO.h

diff --git a/media/mtp/AsyncIO.h b/media/mtp/AsyncIO.h
index f7515a2..ed80828 100644
--- a/media/mtp/AsyncIO.h
+++ b/media/mtp/AsyncIO.h
@@ -48,6 +48,8 @@
     std::thread thread;
     ssize_t ret;
     int error;
+
+    ~aiocb();
 };
 
 // Submit a request for IO to be completed
@@ -58,9 +60,13 @@
 
 // Suspend current thread until given IO is complete, at which point
 // its return value and any errors can be accessed
+// All submitted requests must have a corresponding suspend.
+// aiocb->aio_buf must refer to valid memory until after the suspend call
 int aio_suspend(struct aiocb *[], int, const struct timespec *);
 int aio_error(const struct aiocb *);
 ssize_t aio_return(struct aiocb *);
+
+// (Currently unimplemented)
 int aio_cancel(int, struct aiocb *);
 
 // Initialize a threadpool to perform IO. Only one pool can be

media/mtp/MtpFfsHandle.cpp

diff --git a/media/mtp/MtpFfsHandle.cpp b/media/mtp/MtpFfsHandle.cpp
index 0ea70fa..565a2fe 100644
--- a/media/mtp/MtpFfsHandle.cpp
+++ b/media/mtp/MtpFfsHandle.cpp
@@ -541,14 +541,12 @@
         if (file_length > 0) {
             length = std::min(static_cast<uint32_t>(MAX_FILE_CHUNK_SIZE), file_length);
 
-            // Read data from USB
-            if ((ret = readHandle(mBulkOut, data, length)) == -1) {
-                return -1;
-            }
+            // Read data from USB, handle errors after waiting for write thread.
+            ret = readHandle(mBulkOut, data, length);
 
             if (file_length != MAX_MTP_FILE_SIZE && ret < static_cast<int>(length)) {
+                ret = -1;
                 errno = EIO;
-                return -1;
             }
             read = true;
         }
@@ -569,6 +567,11 @@
             write = false;
         }
 
+        // If there was an error reading above
+        if (ret == -1) {
+            return -1;
+        }
+
         if (read) {
             // Enqueue a new write request
             aio.aio_buf = data;
@@ -626,6 +629,7 @@
     aio.aio_fildes = mfr.fd;
     struct aiocb *aiol[] = {&aio};
     int ret, length;
+    int error = 0;
     bool read = false;
     bool write = false;
 
@@ -669,6 +673,10 @@
             write = true;
         }
 
+        if (error == -1) {
+            return -1;
+        }
+
         if (file_length > 0) {
             length = std::min(static_cast<uint64_t>(MAX_FILE_CHUNK_SIZE), file_length);
             // Queue up another read
@@ -680,8 +688,9 @@
         }
 
         if (write) {
-            if (writeHandle(mBulkIn, data2, ret) == -1)
-                return -1;
+            if (writeHandle(mBulkIn, data2, ret) == -1) {
+                error = -1;
+            }
             write = false;
         }
     }