commit 383f61d9eec276b668bc1853a25c6574582e2547
author Pawin Vongmasa <pawin@google.com> Sat Jul 01 00:03:50 2017 -0700
committer Pawin Vongmasa <pawin@google.com> Sat Jul 01 00:33:06 2017 -0700
tree bdb112a1a14e55e60e3d83d985586ffa7dd4a542
parent d4ef5f1eafa099eb40e68a1dfc7b701e4a8d0b1a

Release the mutex before calling hidl_cb

In Omx::allocateNode, Omx::serviceDied may be called by hidl_cb in
Omx::allocateNode if the observer dies before hidl_cb is called. This CL
prevents a deadlock from happening in this situation.

Test: make cts -j99 && cts-tradefed run cts-dev \
--module CtsMediaTestCases --compatibility:module-arg \
CtsMediaTestCases:include-annotation:\
android.platform.test.annotations.RequiresDevice
Bug: 63177989

Change-Id: I8e1357ff637b0912a3a97c6611d886533caaaf8a

media/libstagefright/omx/1.0/Omx.cpp

diff --git a/media/libstagefright/omx/1.0/Omx.cpp b/media/libstagefright/omx/1.0/Omx.cpp
index 64b2c08..789379a 100644
--- a/media/libstagefright/omx/1.0/Omx.cpp
+++ b/media/libstagefright/omx/1.0/Omx.cpp
@@ -90,46 +90,49 @@
     using ::android::IOMXNode;
     using ::android::IOMXObserver;
 
-    Mutex::Autolock autoLock(mLock);
-    if (mLiveNodes.size() == kMaxNodeInstances) {
-        _hidl_cb(toStatus(NO_MEMORY), nullptr);
-        return Void();
-    }
-
-    sp<OMXNodeInstance> instance = new OMXNodeInstance(
-            this, new LWOmxObserver(observer), name.c_str());
-
-    OMX_COMPONENTTYPE *handle;
-    OMX_ERRORTYPE err = mMaster->makeComponentInstance(
-            name.c_str(), &OMXNodeInstance::kCallbacks,
-            instance.get(), &handle);
-
-    if (err != OMX_ErrorNone) {
-        LOG(ERROR) << "Failed to allocate omx component "
-                "'" << name.c_str() << "' "
-                " err=" << asString(err) <<
-                "(0x" << std::hex << unsigned(err) << ")";
-        _hidl_cb(toStatus(StatusFromOMXError(err)), nullptr);
-        return Void();
-    }
-    instance->setHandle(handle);
-    std::vector<AString> quirkVector;
-    if (mParser.getQuirks(name.c_str(), &quirkVector) == OK) {
-        uint32_t quirks = 0;
-        for (const AString quirk : quirkVector) {
-            if (quirk == "requires-allocate-on-input-ports") {
-                quirks |= kRequiresAllocateBufferOnInputPorts;
-            }
-            if (quirk == "requires-allocate-on-output-ports") {
-                quirks |= kRequiresAllocateBufferOnOutputPorts;
-            }
+    sp<OMXNodeInstance> instance;
+    {
+        Mutex::Autolock autoLock(mLock);
+        if (mLiveNodes.size() == kMaxNodeInstances) {
+            _hidl_cb(toStatus(NO_MEMORY), nullptr);
+            return Void();
         }
-        instance->setQuirks(quirks);
-    }
 
-    mLiveNodes.add(observer.get(), instance);
+        instance = new OMXNodeInstance(
+                this, new LWOmxObserver(observer), name.c_str());
+
+        OMX_COMPONENTTYPE *handle;
+        OMX_ERRORTYPE err = mMaster->makeComponentInstance(
+                name.c_str(), &OMXNodeInstance::kCallbacks,
+                instance.get(), &handle);
+
+        if (err != OMX_ErrorNone) {
+            LOG(ERROR) << "Failed to allocate omx component "
+                    "'" << name.c_str() << "' "
+                    " err=" << asString(err) <<
+                    "(0x" << std::hex << unsigned(err) << ")";
+            _hidl_cb(toStatus(StatusFromOMXError(err)), nullptr);
+            return Void();
+        }
+        instance->setHandle(handle);
+        std::vector<AString> quirkVector;
+        if (mParser.getQuirks(name.c_str(), &quirkVector) == OK) {
+            uint32_t quirks = 0;
+            for (const AString quirk : quirkVector) {
+                if (quirk == "requires-allocate-on-input-ports") {
+                    quirks |= kRequiresAllocateBufferOnInputPorts;
+                }
+                if (quirk == "requires-allocate-on-output-ports") {
+                    quirks |= kRequiresAllocateBufferOnOutputPorts;
+                }
+            }
+            instance->setQuirks(quirks);
+        }
+
+        mLiveNodes.add(observer.get(), instance);
+        mNode2Observer.add(instance.get(), observer.get());
+    }
     observer->linkToDeath(this, 0);
-    mNode2Observer.add(instance.get(), observer.get());
 
     _hidl_cb(toStatus(OK), new TWOmxNode(instance));
     return Void();