From 0cae723c2c41ea0738de634014622ac89ea7b83f Mon Sep 17 00:00:00 2001
From: "Eunki, Hong" <eunkiki.hong@samsung.com>
Date: Wed, 16 Aug 2023 11:53:46 +0900
Subject: [PATCH] Ensure task moved under mutex

There was some logical hole between remove-from-queue and insert-to-queue.
Between remove and insert, the task can be exist without not exist any queue.
If someone request RemoveTask during this timing, task will not be removed.

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

diff --git a/dali/internal/system/common/async-task-manager-impl.cpp b/dali/internal/system/common/async-task-manager-impl.cpp
index 740f365..4b33043 100644
--- a/dali/internal/system/common/async-task-manager-impl.cpp
+++ b/dali/internal/system/common/async-task-manager-impl.cpp
@@ -549,6 +549,9 @@ AsyncTaskPtr AsyncTaskManager::PopNextTaskToProcess()
           auto runningIter = mRunningTasks.insert(mRunningTasks.end(), std::make_pair(nextTask, RunningTaskState::RUNNING));
           CacheImpl::InsertTaskCache(mCacheImpl->mRunningTasksCache, nextTask, runningIter);
 
+          CacheImpl::EraseTaskCache(mCacheImpl->mWaitingTasksCache, nextTask, iter);
+          mWaitingTasks.erase(iter);
+
           // Decrease avaliable task counts if it is low priority
           if(priorityType == AsyncTask::PriorityType::LOW)
           {
@@ -562,9 +565,6 @@ AsyncTaskPtr AsyncTaskManager::PopNextTaskToProcess()
           // Decrease the number of waiting tasks for high priority.
           --mWaitingHighProirityTaskCounts;
         }
-
-        CacheImpl::EraseTaskCache(mCacheImpl->mWaitingTasksCache, nextTask, iter);
-        mWaitingTasks.erase(iter);
         break;
       }
     }
@@ -582,7 +582,9 @@ void AsyncTaskManager::CompleteTask(AsyncTaskPtr&& task)
 
   if(task)
   {
-    // Lock while adding task to the queue
+    const bool needTrigger = task->GetCallbackInvocationThread() == AsyncTask::ThreadType::MAIN_THREAD;
+
+    // Lock while check validation of task.
     {
       Mutex::ScopedLock lock(mRunningTasksMutex);
 
@@ -599,16 +601,6 @@ void AsyncTaskManager::CompleteTask(AsyncTaskPtr&& task)
           // 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)
-        {
-          // We are under running task mutex. We can increase it.
-          ++mAvaliableLowPriorityTaskCounts;
-        }
-        CacheImpl::EraseTaskCache(mCacheImpl->mRunningTasksCache, task, iter);
-        mRunningTasks.erase(iter);
       }
 
       DALI_LOG_INFO(gAsyncTasksManagerLogFilter, Debug::Verbose, "CompleteTask [%p] (is notify? : %d)\n", task.Get(), notify);
@@ -621,21 +613,43 @@ void AsyncTaskManager::CompleteTask(AsyncTaskPtr&& task)
       CallbackBase::Execute(*(task->GetCompletedCallback()), task);
     }
 
-    const bool needTrigger = task->GetCallbackInvocationThread() == AsyncTask::ThreadType::MAIN_THREAD;
-
-    // Move task into completed, for ensure that AsyncTask destroy at main thread.
+    // Lock while adding task to the queue
     {
-      Mutex::ScopedLock lock(mCompletedTasksMutex);
+      Mutex::ScopedLock lock(mRunningTasksMutex);
 
-      const bool callbackRequired = notify && (task->GetCallbackInvocationThread() == AsyncTask::ThreadType::MAIN_THREAD);
+      auto mapIter = mCacheImpl->mRunningTasksCache.find(task.Get());
+      if(mapIter != mCacheImpl->mRunningTasksCache.end())
+      {
+        const auto cacheIter = mapIter->second.begin();
+        DALI_ASSERT_ALWAYS(cacheIter != mapIter->second.end());
+
+        const auto iter         = *cacheIter;
+        const auto priorityType = iter->first->GetPriorityType();
+        // Increase avaliable task counts if it is low priority
+        if(priorityType == AsyncTask::PriorityType::LOW)
+        {
+          // We are under running task mutex. We can increase it.
+          ++mAvaliableLowPriorityTaskCounts;
+        }
+
+        // Move task into completed, for ensure that AsyncTask destroy at main thread.
+        {
+          Mutex::ScopedLock lock(mCompletedTasksMutex); // We can lock this mutex under mRunningTasksMutex.
 
-      DALI_LOG_INFO(gAsyncTasksManagerLogFilter, Debug::Verbose, "Running -> Completed [%p] (callback required? : %d)\n", task.Get(), callbackRequired);
+          const bool callbackRequired = notify && (task->GetCallbackInvocationThread() == AsyncTask::ThreadType::MAIN_THREAD);
 
-      auto completedIter = mCompletedTasks.insert(mCompletedTasks.end(), std::make_pair(task, callbackRequired ? CompletedTaskState::REQUIRE_CALLBACK : CompletedTaskState::SKIP_CALLBACK));
-      CacheImpl::InsertTaskCache(mCacheImpl->mCompletedTasksCache, task, completedIter);
+          DALI_LOG_INFO(gAsyncTasksManagerLogFilter, Debug::Verbose, "Running -> Completed [%p] (callback required? : %d)\n", task.Get(), callbackRequired);
 
-      // Now, task is invalidate.
-      task.Reset();
+          auto completedIter = mCompletedTasks.insert(mCompletedTasks.end(), std::make_pair(task, callbackRequired ? CompletedTaskState::REQUIRE_CALLBACK : CompletedTaskState::SKIP_CALLBACK));
+          CacheImpl::InsertTaskCache(mCacheImpl->mCompletedTasksCache, task, completedIter);
+
+          CacheImpl::EraseTaskCache(mCacheImpl->mRunningTasksCache, task, iter);
+          mRunningTasks.erase(iter);
+
+          // Now, task is invalidate.
+          task.Reset();
+        }
+      }
     }
 
     // Wake up the main thread
-- 
2.7.4