From 494f6f3e07fbc79b3d2ab8b6c3143eb0bcbaebee Mon Sep 17 00:00:00 2001
From: "Eunki, Hong" <eunkiki.hong@samsung.com>
Date: Thu, 28 Dec 2023 13:37:15 +0900
Subject: [PATCH] Ensure to check whether task need to callback or not

There was short-term to unlock the mRunningTasksMutex.
If main thread Remove the running task (mean, cancel) during
that mutex unlocked, we just keep know that
this task need to execute callback.

So, we need to check for notify or not only for lottie cases,
and check main thread callback required one more time for main thread cases.

Change-Id: I8e6bdcdf4b8bf290a23b5b8e3fdb1821db26d393
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
---
 .../system/common/async-task-manager-impl.cpp      | 72 +++++++++++++---------
 1 file changed, 43 insertions(+), 29 deletions(-)

diff --git a/dali/internal/system/common/async-task-manager-impl.cpp b/dali/internal/system/common/async-task-manager-impl.cpp
index 1ff0f02..6590390 100644
--- a/dali/internal/system/common/async-task-manager-impl.cpp
+++ b/dali/internal/system/common/async-task-manager-impl.cpp
@@ -1080,55 +1080,61 @@ AsyncTaskPtr AsyncTaskManager::PopNextTaskToProcess()
 /// Worker thread called
 void AsyncTaskManager::CompleteTask(AsyncTaskPtr&& task)
 {
-  bool notify = false;
-
   if(task)
   {
     bool needTrigger = false;
 
-    // Lock while check validation of task.
+    // Check now whether we need to execute callback or not, for worker thread cases.
+    if(task->GetCallbackInvocationThread() == AsyncTask::ThreadType::WORKER_THREAD)
     {
-      Mutex::ScopedLock lock(mRunningTasksMutex);
+      bool notify = false;
 
-      auto mapIter = mCacheImpl->mRunningTasksCache.find(task.Get());
-      if(mapIter != mCacheImpl->mRunningTasksCache.end())
+      // Lock while check validation of task.
       {
-        const auto cacheIter = mapIter->second.begin();
-        DALI_ASSERT_ALWAYS(cacheIter != mapIter->second.end());
+        Mutex::ScopedLock lock(mRunningTasksMutex);
 
-        const auto iter = *cacheIter;
-        DALI_ASSERT_DEBUG(iter->first == task);
-        if(iter->second == RunningTaskState::RUNNING)
+        auto mapIter = mCacheImpl->mRunningTasksCache.find(task.Get());
+        if(mapIter != mCacheImpl->mRunningTasksCache.end())
         {
-          // This task is valid.
-          notify = true;
-        }
-      }
+          const auto cacheIter = mapIter->second.begin();
+          DALI_ASSERT_ALWAYS(cacheIter != mapIter->second.end());
 
-      DALI_LOG_INFO(gAsyncTasksManagerLogFilter, Debug::Verbose, "CompleteTask [%p][%s] (is notify? : %d)\n", task.Get(), GetTaskName(task), notify);
-    }
+          const auto iter = *cacheIter;
+          DALI_ASSERT_DEBUG(iter->first == task);
+          if(iter->second == RunningTaskState::RUNNING)
+          {
+            // This task is valid.
+            notify = true;
+          }
+        }
 
-    // We should execute this tasks complete callback out of mutex
-    if(notify && task->GetCallbackInvocationThread() == AsyncTask::ThreadType::WORKER_THREAD)
-    {
-      DALI_LOG_INFO(gAsyncTasksManagerLogFilter, Debug::Verbose, "Execute callback on worker thread [%p][%s]\n", task.Get(), GetTaskName(task));
-      CallbackBase::Execute(*(task->GetCompletedCallback()), task);
+        DALI_LOG_INFO(gAsyncTasksManagerLogFilter, Debug::Verbose, "CompleteTask [%p][%s] (is notify? : %d)\n", task.Get(), GetTaskName(task), notify);
+      }
 
-      // We need to remove task trace now.
-      if(mTasksCompletedImpl->IsTasksCompletedCallbackExist())
+      // We should execute this tasks complete callback out of mutex
+      if(notify)
       {
-        mTasksCompletedImpl->RemoveTaskTrace(task);
+        DALI_LOG_INFO(gAsyncTasksManagerLogFilter, Debug::Verbose, "Execute callback on worker thread [%p][%s]\n", task.Get(), GetTaskName(task));
+        CallbackBase::Execute(*(task->GetCompletedCallback()), task);
 
-        if(mTasksCompletedImpl->IsExecuteCallbackExist())
+        // We need to remove task trace now.
+        if(mTasksCompletedImpl->IsTasksCompletedCallbackExist())
         {
-          // We need to call EmitCompletedTasks(). Trigger main thread.
-          needTrigger = true;
+          mTasksCompletedImpl->RemoveTaskTrace(task);
+
+          if(mTasksCompletedImpl->IsExecuteCallbackExist())
+          {
+            // We need to call EmitCompletedTasks(). Trigger main thread.
+            needTrigger = true;
+          }
         }
       }
     }
 
     // Lock while adding task to the queue
     {
+      bool notify = false;
+
       Mutex::ScopedLock lock(mRunningTasksMutex);
 
       auto mapIter = mCacheImpl->mRunningTasksCache.find(task.Get());
@@ -1137,7 +1143,15 @@ void AsyncTaskManager::CompleteTask(AsyncTaskPtr&& task)
         const auto cacheIter = mapIter->second.begin();
         DALI_ASSERT_ALWAYS(cacheIter != mapIter->second.end());
 
-        const auto iter         = *cacheIter;
+        const auto iter = *cacheIter;
+
+        DALI_ASSERT_DEBUG(iter->first == task);
+        if(iter->second == RunningTaskState::RUNNING)
+        {
+          // This task is valid.
+          notify = true;
+        }
+
         const auto priorityType = iter->first->GetPriorityType();
         // Increase avaliable task counts if it is low priority
         if(priorityType == AsyncTask::PriorityType::LOW)
-- 
2.7.4