MediaCodec: fix possible crash at stop & error

If an error occurred during stop() operation but the operation
completed anyway, it may lead to an assert because the error already
sent response to pending reply.

Bug: 154678891
Test: atest CtsMediaTestCases -- --module-arg CtsMediaTestCases:size:small
Test: atest mediacodecTest
Merged-In: Id88d546a22db26495a6d535cbf3e191a21ea21fd
Change-Id: Id88d546a22db26495a6d535cbf3e191a21ea21fd
diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp
index 298a3f9..98b0191 100644
--- a/media/libstagefright/MediaCodec.cpp
+++ b/media/libstagefright/MediaCodec.cpp
@@ -2133,6 +2133,8 @@
                     }
 
                     bool sendErrorResponse = true;
+                    std::string origin{"kWhatError:"};
+                    origin += stateString(mState);
 
                     switch (mState) {
                         case INITIALIZING:
@@ -2184,14 +2186,14 @@
                                 // be a shutdown complete notification after
                                 // all.
 
-                                // note that we're directly going from
+                                // note that we may be directly going from
                                 // STOPPING->UNINITIALIZED, instead of the
                                 // usual STOPPING->INITIALIZED state.
                                 setState(UNINITIALIZED);
                                 if (mState == RELEASING) {
                                     mComponentName.clear();
                                 }
-                                postPendingRepliesAndDeferredMessages();
+                                postPendingRepliesAndDeferredMessages(origin + ":dead");
                                 sendErrorResponse = false;
                             }
                             break;
@@ -2282,7 +2284,7 @@
                             // released by ResourceManager.
                             finalErr = DEAD_OBJECT;
                         }
-                        postPendingRepliesAndDeferredMessages(finalErr);
+                        postPendingRepliesAndDeferredMessages(origin, finalErr);
                     }
                     break;
                 }
@@ -2330,7 +2332,7 @@
                                 MediaResource::CodecResource(mFlags & kFlagIsSecure, mIsVideo));
                     }
 
-                    postPendingRepliesAndDeferredMessages();
+                    postPendingRepliesAndDeferredMessages("kWhatComponentAllocated");
                     break;
                 }
 
@@ -2369,7 +2371,7 @@
                         mFlags |= kFlagUsesSoftwareRenderer;
                     }
                     setState(CONFIGURED);
-                    postPendingRepliesAndDeferredMessages();
+                    postPendingRepliesAndDeferredMessages("kWhatComponentConfigured");
 
                     // augment our media metrics info, now that we know more things
                     // such as what the codec extracted from any CSD passed in.
@@ -2438,7 +2440,7 @@
                     } else {
                         response->setInt32("err", err);
                     }
-                    postPendingRepliesAndDeferredMessages(response);
+                    postPendingRepliesAndDeferredMessages("kWhatInputSurfaceCreated", response);
                     break;
                 }
 
@@ -2460,7 +2462,7 @@
                     } else {
                         response->setInt32("err", err);
                     }
-                    postPendingRepliesAndDeferredMessages(response);
+                    postPendingRepliesAndDeferredMessages("kWhatInputSurfaceAccepted", response);
                     break;
                 }
 
@@ -2478,7 +2480,7 @@
                     if (msg->findInt32("err", &err)) {
                         response->setInt32("err", err);
                     }
-                    postPendingRepliesAndDeferredMessages(response);
+                    postPendingRepliesAndDeferredMessages("kWhatSignaledInputEOS", response);
                     break;
                 }
 
@@ -2497,7 +2499,7 @@
                                 MediaResource::GraphicMemoryResource(getGraphicBufferSize()));
                     }
                     setState(STARTED);
-                    postPendingRepliesAndDeferredMessages();
+                    postPendingRepliesAndDeferredMessages("kWhatStartCompleted");
                     break;
                 }
 
@@ -2633,7 +2635,13 @@
                         break;
                     }
                     setState(INITIALIZED);
-                    postPendingRepliesAndDeferredMessages();
+                    if (mReplyID) {
+                        postPendingRepliesAndDeferredMessages("kWhatStopCompleted");
+                    } else {
+                        ALOGW("kWhatStopCompleted: presumably an error occurred earlier, "
+                              "but the operation completed anyway. (last reply origin=%s)",
+                              mLastReplyOrigin.c_str());
+                    }
                     break;
                 }
 
@@ -2657,7 +2665,7 @@
                     mReleaseSurface.reset();
 
                     if (mReplyID != nullptr) {
-                        postPendingRepliesAndDeferredMessages();
+                        postPendingRepliesAndDeferredMessages("kWhatReleaseCompleted");
                     }
                     if (mAsyncReleaseCompleteNotification != nullptr) {
                         flushMediametrics();
@@ -2682,7 +2690,7 @@
                         mCodec->signalResume();
                     }
 
-                    postPendingRepliesAndDeferredMessages();
+                    postPendingRepliesAndDeferredMessages("kWhatFlushCompleted");
                     break;
                 }
 
@@ -3071,7 +3079,8 @@
             if (mState == FLUSHING || mState == STOPPING
                     || mState == CONFIGURING || mState == STARTING) {
                 // mReply is always set if in these states.
-                postPendingRepliesAndDeferredMessages();
+                postPendingRepliesAndDeferredMessages(
+                        std::string("kWhatRelease:") + stateString(mState));
             }
 
             if (mFlags & kFlagSawMediaServerDie) {
@@ -3120,7 +3129,7 @@
                 // State transition replies are handled above, so this reply
                 // would not be related to state transition. As we are
                 // shutting down the component, just fail the operation.
-                postPendingRepliesAndDeferredMessages(UNKNOWN_ERROR);
+                postPendingRepliesAndDeferredMessages("kWhatRelease:reply", UNKNOWN_ERROR);
             }
             mReplyID = replyID;
             setState(msg->what() == kWhatStop ? STOPPING : RELEASING);
@@ -3136,7 +3145,7 @@
 
             if (asyncNotify != nullptr) {
                 mResourceManagerProxy->markClientForPendingRemoval();
-                postPendingRepliesAndDeferredMessages();
+                postPendingRepliesAndDeferredMessages("kWhatRelease:async");
                 asyncNotifyPost.clear();
                 mAsyncReleaseCompleteNotification = asyncNotify;
             }
@@ -4323,16 +4332,23 @@
     return OK;
 }
 
-void MediaCodec::postPendingRepliesAndDeferredMessages(status_t err /* = OK */) {
+void MediaCodec::postPendingRepliesAndDeferredMessages(
+        std::string origin, status_t err /* = OK */) {
     sp<AMessage> response{new AMessage};
     if (err != OK) {
         response->setInt32("err", err);
     }
-    postPendingRepliesAndDeferredMessages(response);
+    postPendingRepliesAndDeferredMessages(origin, response);
 }
 
-void MediaCodec::postPendingRepliesAndDeferredMessages(const sp<AMessage> &response) {
-    CHECK(mReplyID);
+void MediaCodec::postPendingRepliesAndDeferredMessages(
+        std::string origin, const sp<AMessage> &response) {
+    LOG_ALWAYS_FATAL_IF(
+            !mReplyID,
+            "postPendingRepliesAndDeferredMessages: mReplyID == null, from %s following %s",
+            origin.c_str(),
+            mLastReplyOrigin.c_str());
+    mLastReplyOrigin = origin;
     response->postReply(mReplyID);
     mReplyID.clear();
     ALOGV_IF(!mDeferredMessages.empty(),
diff --git a/media/libstagefright/include/media/stagefright/MediaCodec.h b/media/libstagefright/include/media/stagefright/MediaCodec.h
index 823677a..7614ba5 100644
--- a/media/libstagefright/include/media/stagefright/MediaCodec.h
+++ b/media/libstagefright/include/media/stagefright/MediaCodec.h
@@ -366,6 +366,7 @@
     AString mOwnerName;
     sp<MediaCodecInfo> mCodecInfo;
     sp<AReplyToken> mReplyID;
+    std::string mLastReplyOrigin;
     std::vector<sp<AMessage>> mDeferredMessages;
     uint32_t mFlags;
     status_t mStickyError;
@@ -491,8 +492,8 @@
     bool hasPendingBuffer(int portIndex);
     bool hasPendingBuffer();
 
-    void postPendingRepliesAndDeferredMessages(status_t err = OK);
-    void postPendingRepliesAndDeferredMessages(const sp<AMessage> &response);
+    void postPendingRepliesAndDeferredMessages(std::string origin, status_t err = OK);
+    void postPendingRepliesAndDeferredMessages(std::string origin, const sp<AMessage> &response);
 
     /* called to get the last codec error when the sticky flag is set.
      * if no such codec error is found, returns UNKNOWN_ERROR.
diff --git a/media/libstagefright/tests/mediacodec/Android.bp b/media/libstagefright/tests/mediacodec/Android.bp
index 006864e..0bd0639 100644
--- a/media/libstagefright/tests/mediacodec/Android.bp
+++ b/media/libstagefright/tests/mediacodec/Android.bp
@@ -23,7 +23,12 @@
         "MediaTestHelper.cpp",
     ],
 
+    header_libs: [
+        "libmediadrm_headers",
+    ],
+
     shared_libs: [
+        "libgui",
         "libmedia",
         "libmedia_codeclist",
         "libmediametrics",
diff --git a/media/libstagefright/tests/mediacodec/MediaCodecTest.cpp b/media/libstagefright/tests/mediacodec/MediaCodecTest.cpp
index baa86c1..d00a50f 100644
--- a/media/libstagefright/tests/mediacodec/MediaCodecTest.cpp
+++ b/media/libstagefright/tests/mediacodec/MediaCodecTest.cpp
@@ -20,6 +20,8 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <gui/Surface.h>
+#include <mediadrm/ICrypto.h>
 #include <media/stagefright/CodecBase.h>
 #include <media/stagefright/MediaCodec.h>
 #include <media/stagefright/MediaCodecListWriter.h>
@@ -152,6 +154,37 @@
 using namespace android;
 using ::testing::_;
 
+static sp<MediaCodec> SetupMediaCodec(
+        const AString &owner,
+        const AString &codecName,
+        const AString &mediaType,
+        const sp<ALooper> &looper,
+        std::function<sp<CodecBase>(const AString &name, const char *owner)> getCodecBase) {
+    std::shared_ptr<MediaCodecListWriter> listWriter =
+        MediaTestHelper::CreateCodecListWriter();
+    std::unique_ptr<MediaCodecInfoWriter> infoWriter = listWriter->addMediaCodecInfo();
+    infoWriter->setName(codecName.c_str());
+    infoWriter->setOwner(owner.c_str());
+    infoWriter->addMediaType(mediaType.c_str());
+    std::vector<sp<MediaCodecInfo>> codecInfos;
+    MediaTestHelper::WriteCodecInfos(listWriter, &codecInfos);
+    std::function<status_t(const AString &, sp<MediaCodecInfo> *)> getCodecInfo =
+        [codecInfos](const AString &name, sp<MediaCodecInfo> *info) -> status_t {
+            auto it = std::find_if(
+                    codecInfos.begin(), codecInfos.end(),
+                    [&name](const sp<MediaCodecInfo> &info) {
+                        return name.equalsIgnoreCase(info->getCodecName());
+                    });
+
+            *info = (it == codecInfos.end()) ? nullptr : *it;
+            return (*info) ? OK : NAME_NOT_FOUND;
+        };
+
+    looper->start();
+    return MediaTestHelper::CreateCodec(
+            codecName, looper, getCodecBase, getCodecInfo);
+}
+
 TEST(MediaCodecTest, ReclaimReleaseRace) {
     // Test scenario:
     //
@@ -202,30 +235,9 @@
             return mockCodec;
         };
 
-    std::shared_ptr<MediaCodecListWriter> listWriter =
-        MediaTestHelper::CreateCodecListWriter();
-    std::unique_ptr<MediaCodecInfoWriter> infoWriter = listWriter->addMediaCodecInfo();
-    infoWriter->setName(kCodecName.c_str());
-    infoWriter->setOwner(kCodecOwner.c_str());
-    infoWriter->addMediaType(kMediaType.c_str());
-    std::vector<sp<MediaCodecInfo>> codecInfos;
-    MediaTestHelper::WriteCodecInfos(listWriter, &codecInfos);
-    std::function<status_t(const AString &, sp<MediaCodecInfo> *)> getCodecInfo =
-        [codecInfos](const AString &name, sp<MediaCodecInfo> *info) -> status_t {
-            auto it = std::find_if(
-                    codecInfos.begin(), codecInfos.end(),
-                    [&name](const sp<MediaCodecInfo> &info) {
-                        return name.equalsIgnoreCase(info->getCodecName());
-                    });
-
-            *info = (it == codecInfos.end()) ? nullptr : *it;
-            return (*info) ? OK : NAME_NOT_FOUND;
-        };
-
     sp<ALooper> looper{new ALooper};
-    looper->start();
-    sp<MediaCodec> codec = MediaTestHelper::CreateCodec(
-            kCodecName, looper, getCodecBase, getCodecInfo);
+    sp<MediaCodec> codec = SetupMediaCodec(
+            kCodecOwner, kCodecName, kMediaType, looper, getCodecBase);
     ASSERT_NE(nullptr, codec) << "Codec must not be null";
     ASSERT_NE(nullptr, mockCodec) << "MockCodec must not be null";
     std::promise<void> reclaimCompleted;
@@ -266,3 +278,73 @@
         << "release timed out";
     looper->stop();
 }
+
+TEST(MediaCodecTest, ErrorWhileStopping) {
+    // Test scenario:
+    //
+    // 1) Client thread calls stop(); MediaCodec looper thread calls
+    //    initiateShutdown(); shutdown is being handled at the component thread.
+    // 2) Error occurred, but the shutdown operation is still being done.
+    // 3) MediaCodec looper thread handles the error.
+    // 4) Component thread completes shutdown and posts onStopCompleted()
+
+    static const AString kCodecName{"test.codec"};
+    static const AString kCodecOwner{"nobody"};
+    static const AString kMediaType{"video/x-test"};
+
+    std::promise<void> errorOccurred;
+    sp<MockCodec> mockCodec;
+    std::function<sp<CodecBase>(const AString &name, const char *owner)> getCodecBase =
+        [&mockCodec, &errorOccurred](const AString &, const char *) {
+            mockCodec = new MockCodec([](const std::shared_ptr<MockBufferChannel> &) {
+                // No mock setup, as we don't expect any buffer operations
+                // in this scenario.
+            });
+            ON_CALL(*mockCodec, initiateAllocateComponent(_))
+                .WillByDefault([mockCodec](const sp<AMessage> &) {
+                    mockCodec->callback()->onComponentAllocated(kCodecName.c_str());
+                });
+            ON_CALL(*mockCodec, initiateConfigureComponent(_))
+                .WillByDefault([mockCodec](const sp<AMessage> &msg) {
+                    mockCodec->callback()->onComponentConfigured(
+                            msg->dup(), msg->dup());
+                });
+            ON_CALL(*mockCodec, initiateStart())
+                .WillByDefault([mockCodec]() {
+                    mockCodec->callback()->onStartCompleted();
+                });
+            ON_CALL(*mockCodec, initiateShutdown(true))
+                .WillByDefault([mockCodec, &errorOccurred](bool) {
+                    mockCodec->callback()->onError(UNKNOWN_ERROR, ACTION_CODE_FATAL);
+                    // Mark that 1) and 2) are complete.
+                    errorOccurred.set_value();
+                });
+            ON_CALL(*mockCodec, initiateShutdown(false))
+                .WillByDefault([mockCodec](bool) {
+                    mockCodec->callback()->onReleaseCompleted();
+                });
+            return mockCodec;
+        };
+
+    sp<ALooper> looper{new ALooper};
+    sp<MediaCodec> codec = SetupMediaCodec(
+            kCodecOwner, kCodecName, kMediaType, looper, getCodecBase);
+    ASSERT_NE(nullptr, codec) << "Codec must not be null";
+    ASSERT_NE(nullptr, mockCodec) << "MockCodec must not be null";
+
+    std::thread([mockCodec, &errorOccurred]{
+        // Simulate component thread that handles stop()
+        errorOccurred.get_future().wait();
+        // Error occurred but shutdown request still got processed.
+        mockCodec->callback()->onStopCompleted();
+    }).detach();
+
+    codec->configure(new AMessage, nullptr, nullptr, 0);
+    codec->start();
+    codec->stop();
+    // Sleep here to give time for the MediaCodec looper thread
+    // to process the messages.
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+    codec->release();
+    looper->stop();
+}