C2BqBuffer: resolve 3 way deadlock

Do not hold lock when IPC call is expected from HAL.

C2SurfaceSyncObj is shared lock between framework and HAL. HAL process
can have only one thread to handle IPC from HAL to framework.
Therefore Holding C2SurfaceSyncObj from HAL during IPC call could
trigger deadlock. The exact scenario is as follows.

Thread #1:(HAL -> framework IPC) HIDL call onInputBuffersReleased()
            calls to feedInputBufferIfAvailable(). Since this is using
            HAL IPC thread, this will block Thread #3. This is waiting
            for mOuput mutex which is held by Thread #2.
Thread #2:(framework) discardBuffer() holds mOutput mutex which blocks
            Thread #1. But this is waiting for C2SurfaceSyncObj which is
            held by Thread #3.
Thread #3:(HAL) Dtor of C2BufferQueueBlockPoolData is holding
            C2SurfaceSyncObj, therefore this will block #2. This thread
            is waiting for HIDL IPC thread to be free in order for
            'igbp->cancel()', but HIDL IPC thread is already occupied by
            Thread #1.

Bug: 246707566
Test: atest android.media.decoder.cts.AdaptivePlaybackTest
Test: atest android.media.decoder.cts.DecoderTest
Change-Id: I6a9540d3b4d03806cd40bb4f89a234a6b77758a9
Merged-In: I6a9540d3b4d03806cd40bb4f89a234a6b77758a9

media/codec2/vndk/include/C2SurfaceSyncObj.h

@@ -72,12 +72,13 @@ struct C2SyncVariables {
     /**
      * Notify a buffer is queued. Return whether the upcoming dequeue operation
      * is not blocked. if it's blocked and waitId is non-null, waitId is returned
-     * to be used for waiting.
+     * to be used for waiting. Notify(wake-up) waitors only when 'notify' is
+     * true.
      *
      * \retval false    dequeue operation is blocked now.
      * \retval true     dequeue operation is possible.
      */
-    bool notifyQueuedLocked(uint32_t *waitId = nullptr);
+    bool notifyQueuedLocked(uint32_t *waitId = nullptr, bool notify = true);
 
     /**
      * Notify a buffer is dequeued.

media/codec2/vndk/platform/C2BqBuffer.cpp

@@ -436,8 +436,8 @@ class C2BufferQueueBlockPool::Impl
             if (status == -ETIME) {
                 // fence is not signalled yet.
                 if (syncVar) {
-                    syncVar->lock();
                     (void)mProducer->cancelBuffer(slot, hFenceWrapper.getHandle()).isOk();
+                    syncVar->lock();
                     dequeueable = syncVar->notifyQueuedLocked(&waitId);
                     syncVar->unlock();
                     if (c2Fence) {
@@ -452,8 +452,8 @@ class C2BufferQueueBlockPool::Impl
             if (status != android::NO_ERROR) {
                 ALOGD("buffer fence wait error %d", status);
                 if (syncVar) {
-                    syncVar->lock();
                     (void)mProducer->cancelBuffer(slot, hFenceWrapper.getHandle()).isOk();
+                    syncVar->lock();
                     syncVar->notifyQueuedLocked();
                     syncVar->unlock();
                     if (c2Fence) {
@@ -502,8 +502,8 @@ class C2BufferQueueBlockPool::Impl
             } else if (status != android::NO_ERROR) {
                 slotBuffer.clear();
                 if (syncVar) {
-                    syncVar->lock();
                     (void)mProducer->cancelBuffer(slot, hFenceWrapper.getHandle()).isOk();
+                    syncVar->lock();
                     syncVar->notifyQueuedLocked();
                     syncVar->unlock();
                     if (c2Fence) {
@@ -550,8 +550,8 @@ class C2BufferQueueBlockPool::Impl
             // Block was not created. call requestBuffer# again next time.
             slotBuffer.clear();
             if (syncVar) {
-                syncVar->lock();
                 (void)mProducer->cancelBuffer(slot, hFenceWrapper.getHandle()).isOk();
+                syncVar->lock();
                 syncVar->notifyQueuedLocked();
                 syncVar->unlock();
                 if (c2Fence) {
@@ -813,11 +813,10 @@ C2BufferQueueBlockPoolData::~C2BufferQueueBlockPoolData() {
         if (mGeneration == mCurrentGeneration && mBqId == mCurrentBqId && !mOwner.expired()) {
             C2SyncVariables *syncVar = mSyncMem ? mSyncMem->mem() : nullptr;
             if (syncVar) {
+                mIgbp->cancelBuffer(mBqSlot, hidl_handle{}).isOk();
                 syncVar->lock();
-                if (syncVar->getSyncStatusLocked() == C2SyncVariables::STATUS_ACTIVE) {
-                    mIgbp->cancelBuffer(mBqSlot, hidl_handle{}).isOk();
-                    syncVar->notifyQueuedLocked();
-                }
+                syncVar->notifyQueuedLocked(nullptr,
+                        syncVar->getSyncStatusLocked() == C2SyncVariables::STATUS_ACTIVE);
                 syncVar->unlock();
             } else {
                 mIgbp->cancelBuffer(mBqSlot, hidl_handle{}).isOk();
@@ -826,11 +825,10 @@ C2BufferQueueBlockPoolData::~C2BufferQueueBlockPoolData() {
     } else if (!mOwner.expired()) {
         C2SyncVariables *syncVar = mSyncMem ? mSyncMem->mem() : nullptr;
         if (syncVar) {
+            mIgbp->cancelBuffer(mBqSlot, hidl_handle{}).isOk();
             syncVar->lock();
-            if (syncVar->getSyncStatusLocked() != C2SyncVariables::STATUS_SWITCHING) {
-                mIgbp->cancelBuffer(mBqSlot, hidl_handle{}).isOk();
-                syncVar->notifyQueuedLocked();
-            }
+            syncVar->notifyQueuedLocked(nullptr,
+                    syncVar->getSyncStatusLocked() != C2SyncVariables::STATUS_SWITCHING);
             syncVar->unlock();
         } else {
             mIgbp->cancelBuffer(mBqSlot, hidl_handle{}).isOk();

media/codec2/vndk/platform/C2SurfaceSyncObj.cpp

@@ -177,12 +177,14 @@ bool C2SyncVariables::isDequeueableLocked(uint32_t *waitId) {
     return true;
 }
 
-bool C2SyncVariables::notifyQueuedLocked(uint32_t *waitId) {
+bool C2SyncVariables::notifyQueuedLocked(uint32_t *waitId, bool notify) {
     // Note. thundering herds may occur. Edge trigged signalling.
     // But one waiter will guarantee to dequeue. others may wait again.
     // Minimize futex syscall(trap) for the main use case(one waiter case).
     if (mMaxDequeueCount == mCurDequeueCount--) {
-        broadcast();
+        if (notify) {
+            broadcast();
+        }
         return true;
     }