commit e40c74f14483be48fb1e9030692eb0064c1861ad
author Avinash Malipatil <avinashmp@google.com> Fri Nov 24 02:53:24 2023 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Fri Nov 24 02:53:24 2023 +0000
tree 809cd68bf0720b14dcd47436af6191a21e9bdbe4
parent b32168ebf5b720cf5b9479c5011028f44c7b39cb
parent b61cf9f7367ebebf2d1617e432eb759db1f8ef67

Merge "Fix timer and RtcpEncoder mutex nesting" into udc-qpr-dev am: 27467d304b am: b61cf9f736

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/modules/ImsMedia/+/25418378

Change-Id: Ib1af48a14516d858ac653f5361152b21a40524cd
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/nodes/RtcpEncoderNode.cpp

diff --git a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/nodes/RtcpEncoderNode.cpp b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/nodes/RtcpEncoderNode.cpp
index 35b4acb..cbdc703 100644
--- a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/nodes/RtcpEncoderNode.cpp
+++ b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/nodes/RtcpEncoderNode.cpp
@@ -92,13 +92,6 @@
 void RtcpEncoderNode::Stop()
 {
     IMLOGD0("[Stop]");
-    std::lock_guard<std::mutex> guard(mMutexTimer);
-
-    if (mRtpSession != nullptr)
-    {
-        mRtpSession->StopRtcp();
-    }
-
     if (mTimer != nullptr)
     {
         ImsMediaTimer::TimerStop(mTimer, nullptr);
@@ -106,7 +99,17 @@
         IMLOGD0("[Stop] Rtcp Timer stopped");
     }
 
-    mNodeState = kNodeStateStopped;
+    {
+        std::lock_guard<std::mutex> guard(mMutexTimer);
+        IMLOGD0("[Stop] mutex taken");
+
+        if (mRtpSession != nullptr)
+        {
+            mRtpSession->StopRtcp();
+        }
+
+        mNodeState = kNodeStateStopped;
+    }
 }
 
 bool RtcpEncoderNode::IsRunTime()

service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/utils/ImsMediaTimer.cpp

diff --git a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/utils/ImsMediaTimer.cpp b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/utils/ImsMediaTimer.cpp
index 287a358..143e64f 100644
--- a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/utils/ImsMediaTimer.cpp
+++ b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/utils/ImsMediaTimer.cpp
@@ -37,9 +37,9 @@
     bool mTerminateThread;
     uint32_t mStartTimeSec;
     uint32_t mStartTimeMSec;
+    std::mutex mMutex;
 };
 
-static std::mutex gMutex;
 static std::mutex gMutexList;
 static std::list<TimerInstance*> gTimerList;
 
@@ -135,21 +135,19 @@
                     pInstance->mStartTimeMSec = nCurrTimeMSec;
                 }
 
-                gMutex.lock();
+                {  // Critical section
+                    std::lock_guard<std::mutex> guard(pInstance->mMutex);
+                    if (pInstance->mTerminateThread)
+                    {
+                        break;
+                    }
 
-                if (pInstance->mTerminateThread)
-                {
-                    gMutex.unlock();
-                    break;
+                    if (pInstance->mTimerCb)
+                    {
+                        pInstance->mTimerCb(pInstance, pInstance->mUserData);
+                    }
                 }
 
-                if (pInstance->mTimerCb)
-                {
-                    pInstance->mTimerCb(pInstance, pInstance->mUserData);
-                }
-
-                gMutex.unlock();
-
                 if (pInstance->mRepeat == false)
                 {
                     break;
@@ -162,7 +160,7 @@
 
     if (pInstance != nullptr)
     {
-        free(pInstance);
+        delete pInstance;
         pInstance = nullptr;
     }
 
@@ -173,7 +171,7 @@
         uint32_t nDuration, bool bRepeat, fn_TimerCb pTimerCb, void* pUserData)
 {
     struct timeval tp;
-    TimerInstance* pInstance = reinterpret_cast<TimerInstance*>(malloc(sizeof(TimerInstance)));
+    TimerInstance* pInstance = new TimerInstance;
 
     if (pInstance == nullptr)
     {
@@ -186,7 +184,7 @@
     pInstance->mUserData = pUserData;
     pInstance->mTerminateThread = false;
 
-    IMLOGD3("[TimerStart] Duratation[%u], bRepeat[%d], pUserData[%x]", pInstance->mDuration,
+    IMLOGD3("[TimerStart] Duration[%u], bRepeat[%d], pUserData[%x]", pInstance->mDuration,
             bRepeat, pInstance->mUserData);
 
     if (gettimeofday(&tp, nullptr) != -1)
@@ -196,7 +194,7 @@
     }
     else
     {
-        free(pInstance);
+        delete pInstance;
         return nullptr;
     }
 
@@ -211,6 +209,7 @@
 {
     TimerInstance* pInstance = reinterpret_cast<TimerInstance*>(hTimer);
 
+    IMLOGD1("[TimerStop] pInstance[%x]", pInstance);
     if (pInstance == nullptr)
     {
         return false;
@@ -221,15 +220,18 @@
         return false;
     }
 
-    gMutex.lock();  // just wait until timer callback returns...
-    pInstance->mTerminateThread = true;
-
-    if (ppUserData)
     {
-        *ppUserData = pInstance->mUserData;
+        std::lock_guard<std::mutex> guard(pInstance->mMutex);
+        IMLOGD1("[TimerStop] mutex taken pInstance[%x]", pInstance);
+
+        pInstance->mTerminateThread = true;
+
+        if (ppUserData)
+        {
+            *ppUserData = pInstance->mUserData;
+        }
     }
 
-    gMutex.unlock();
     return true;
 }