commit 9bd3c9b0e894c9f6c48a2cfa7c5f21d384b1c596
author Andy Hung <hunga@google.com> Wed Sep 07 14:42:55 2016 -0700
committer Andy Hung <hunga@google.com> Thu Sep 08 20:09:54 2016 -0700
tree a09214342f1a34d1b2d1188dc1fe55fd172994aa
parent 57d8e70186106995ec0ae0891f014a3209f71551

Improve MediaBuffer robustness for remote clients

Allow remote process to die or behave incorrectly.

Bug: 31060086
Change-Id: I01bc8984287fed61a46083ec090b7773e49158a9

media/libmedia/IMediaSource.cpp

diff --git a/media/libmedia/IMediaSource.cpp b/media/libmedia/IMediaSource.cpp
index dd94ccf..5289c5f 100644
--- a/media/libmedia/IMediaSource.cpp
+++ b/media/libmedia/IMediaSource.cpp
@@ -58,9 +58,9 @@
 
 protected:
     virtual ~RemoteMediaBufferWrapper() {
-        // Indicate to MediaBufferGroup to release.
-        int32_t old = addPendingRelease(1);
-        ALOGV("RemoteMediaBufferWrapper: releasing %p, old %d", this, old);
+        // Release our interest in the MediaBuffer's shared memory.
+        int32_t old = addRemoteRefcount(-1);
+        ALOGV("RemoteMediaBufferWrapper: releasing %p, refcount %d", this, old - 1);
         mMemory.clear(); // don't set the dead object flag.
     }
 };
@@ -296,8 +296,8 @@
         case STOP: {
             ALOGV("stop");
             CHECK_INTERFACE(IMediaSource, data, reply);
+            mGroup->signalBufferReturned(nullptr);
             status_t status = stop();
-            mGroup->gc();
             mIndexCache.reset();
             mBuffersSinceStop = 0;
             return status;
@@ -305,6 +305,7 @@
         case PAUSE: {
             ALOGV("pause");
             CHECK_INTERFACE(IMediaSource, data, reply);
+            mGroup->signalBufferReturned(nullptr);
             return pause();
         }
         case GETFORMAT: {
@@ -336,7 +337,7 @@
                     && len == sizeof(opts)
                     && data.read((void *)&opts, len) == NO_ERROR;
 
-            mGroup->gc(kBinderMediaBuffers /* freeBuffers */);
+            mGroup->signalBufferReturned(nullptr);
             mIndexCache.gc();
             size_t inlineTransferSize = 0;
             status_t ret = NO_ERROR;
@@ -411,10 +412,11 @@
                     reply->writeInt32(offset);
                     reply->writeInt32(length);
                     buf->meta_data()->writeToParcel(*reply);
-                    if (transferBuf != buf) {
-                        buf->release();
-                    } else if (!supportNonblockingRead()) {
-                        maxNumBuffers = 0; // stop readMultiple with one shared buffer.
+                    if (transferBuf == buf) {
+                        buf->addRemoteRefcount(1);
+                        if (!supportNonblockingRead()) {
+                            maxNumBuffers = 0; // stop readMultiple with one shared buffer.
+                        }
                     }
                 } else {
                     ALOGV_IF(buf->mMemory != nullptr,
@@ -423,12 +425,12 @@
                     reply->writeInt32(INLINE_BUFFER);
                     reply->writeByteArray(length, (uint8_t*)buf->data() + offset);
                     buf->meta_data()->writeToParcel(*reply);
-                    buf->release();
                     inlineTransferSize += length;
                     if (inlineTransferSize > kInlineMaxTransfer) {
                         maxNumBuffers = 0; // stop readMultiple if inline transfer is too large.
                     }
                 }
+                buf->release();
             }
             reply->writeInt32(NULL_BUFFER); // Indicate no more MediaBuffers.
             reply->writeInt32(ret);