From be6a118e435fbddc0c14f04e5ce805f0c51756ac Mon Sep 17 00:00:00 2001
From: Sungtak Lee <taklee@google.com>
Date: Mon, 17 Dec 2018 19:00:40 -0800
Subject: [PATCH] bufferpool2.0: Avoid lock during hidl oneway call

Hidl oneway call works as synchronous call when it is called from same
process. Avoid lock while calling hidl oneway interfaces.

Bug: 121047202
Change-Id: I20c29640414edd70e414af749c0b3f96efda8ca3
---
 media/bufferpool/2.0/AccessorImpl.cpp | 32 +++++++++++++++++++--------
 media/bufferpool/2.0/AccessorImpl.h   |  4 +++-
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/media/bufferpool/2.0/AccessorImpl.cpp b/media/bufferpool/2.0/AccessorImpl.cpp
index 2c734ac565..526090943c 100644
--- a/media/bufferpool/2.0/AccessorImpl.cpp
+++ b/media/bufferpool/2.0/AccessorImpl.cpp
@@ -253,9 +253,21 @@ void Accessor::Impl::flush() {
 }
 
 void Accessor::Impl::handleInvalidateAck() {
-    std::lock_guard<std::mutex> lock(mBufferPool.mMutex);
-    mBufferPool.processStatusMessages();
-    mBufferPool.mInvalidation.onHandleAck();
+    std::map<ConnectionId, const sp<IObserver>> observers;
+    uint32_t invalidationId;
+    {
+        std::lock_guard<std::mutex> lock(mBufferPool.mMutex);
+        mBufferPool.processStatusMessages();
+        mBufferPool.mInvalidation.onHandleAck(&observers, &invalidationId);
+    }
+    // Do not hold lock for send invalidations
+    for (auto it = observers.begin(); it != observers.end(); ++it) {
+        const sp<IObserver> observer = it->second;
+        if (observer) {
+            Return<void> transResult = observer->onMessage(it->first, invalidationId);
+            (void) transResult;
+        }
+    }
 }
 
 bool Accessor::Impl::isValid() {
@@ -365,19 +377,21 @@ void Accessor::Impl::BufferPool::Invalidation::onInvalidationRequest(
     sInvalidator->addAccessor(mId, impl);
 }
 
-void Accessor::Impl::BufferPool::Invalidation::onHandleAck() {
+void Accessor::Impl::BufferPool::Invalidation::onHandleAck(
+        std::map<ConnectionId, const sp<IObserver>> *observers,
+        uint32_t *invalidationId) {
     if (mInvalidationId != 0) {
+        *invalidationId = mInvalidationId;
         std::set<int> deads;
         for (auto it = mAcks.begin(); it != mAcks.end(); ++it) {
             if (it->second != mInvalidationId) {
                 const sp<IObserver> observer = mObservers[it->first];
                 if (observer) {
-                    ALOGV("connection %lld call observer (%u: %u)",
+                    observers->emplace(it->first, observer);
+                    ALOGV("connection %lld will call observer (%u: %u)",
                           (long long)it->first, it->second, mInvalidationId);
-                    Return<void> transResult = observer->onMessage(it->first, mInvalidationId);
-                    (void) transResult;
-                    // N.B: ignore possibility of onMessage oneway call being
-                    // lost.
+                    // N.B: onMessage will be called later. ignore possibility of
+                    // onMessage# oneway call being lost.
                     it->second = mInvalidationId;
                 } else {
                     ALOGV("bufferpool2 observer died %lld", (long long)it->first);
diff --git a/media/bufferpool/2.0/AccessorImpl.h b/media/bufferpool/2.0/AccessorImpl.h
index b3faa962a9..eea72b98fb 100644
--- a/media/bufferpool/2.0/AccessorImpl.h
+++ b/media/bufferpool/2.0/AccessorImpl.h
@@ -158,7 +158,9 @@ private:
                     BufferInvalidationChannel &channel,
                     const std::shared_ptr<Accessor::Impl> &impl);
 
-            void onHandleAck();
+            void onHandleAck(
+                    std::map<ConnectionId, const sp<IObserver>> *observers,
+                    uint32_t *invalidationId);
         } mInvalidation;
         /// Buffer pool statistics which tracks allocation and transfer statistics.
         struct Stats {