From: Eunki, Hong <eunkiki.hong@samsung.com>
Date: Wed, 14 Aug 2024 01:49:50 +0000 (+0900)
Subject: [Tizen] (Vector) Make SleepThread more thread safe enough
X-Git-Tag: accepted/tizen/8.0/unified/20240823.183219~1
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a90d3bbf1618ceb4be4499eeeea1cbba1bf5cb95;p=platform%2Fcore%2Fuifw%2Fdali-toolkit.git

[Tizen] (Vector) Make SleepThread more thread safe enough

Make sleep thread destroyed more early time.
It will make that sleepthread cannot call invalid mAwakeCallback.

Also, Make more thread safety when we change the timepoint and sleep

Change-Id: I0ce82503576efff045a45dfdec219f483c08ba03
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
---

diff --git a/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.cpp b/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.cpp
index 8a4ce55f58..e4c8ebd4b4 100644
--- a/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.cpp
+++ b/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.cpp
@@ -84,6 +84,9 @@ VectorAnimationThread::~VectorAnimationThread()
 
   DALI_LOG_INFO(gVectorAnimationLogFilter, Debug::Verbose, "VectorAnimationThread::~VectorAnimationThread: Join [%p]\n", this);
 
+  // Mark as sleep thread destroyed now.
+  mSleepThread.Finalize();
+
   DALI_LOG_DEBUG_INFO("VectorAnimationThread Join request\n");
 
   Join();
@@ -120,14 +123,16 @@ void VectorAnimationThread::OnTaskCompleted(VectorAnimationTaskPtr task, bool su
   }
 }
 
-/// VectorAnimationThread::SleepThread called
+/// VectorAnimationThread::SleepThread called, Mutex SleepThread::mAwakeCallbackMutex is locked
 void VectorAnimationThread::OnAwakeFromSleep()
 {
   if(DALI_LIKELY(!mDestroyThread))
   {
+    ConditionalWait::ScopedLock lock(mConditionalWait);
+
     mNeedToSleep = false;
     // wake up the animation thread
-    mConditionalWait.Notify();
+    mConditionalWait.Notify(lock);
   }
 }
 
@@ -412,11 +417,13 @@ VectorAnimationThread::SleepThread::~SleepThread()
   // Stop the thread
   {
     ConditionalWait::ScopedLock lock(mConditionalWait);
-    mDestroyThread = true;
-    mAwakeCallback.reset();
+    Finalize();
+
     mConditionalWait.Notify(lock);
   }
 
+  DALI_LOG_DEBUG_INFO("VectorAnimationThread::SleepThread Join request\n");
+
   Join();
 }
 
@@ -424,9 +431,27 @@ VectorAnimationThread::SleepThread::~SleepThread()
 void VectorAnimationThread::SleepThread::SleepUntil(std::chrono::time_point<std::chrono::steady_clock> timeToSleepUntil)
 {
   ConditionalWait::ScopedLock lock(mConditionalWait);
-  mSleepTimePoint = timeToSleepUntil;
-  mNeedToSleep    = true;
-  mConditionalWait.Notify(lock);
+
+  Mutex::ScopedLock sleepLock(mSleepRequestMutex);
+
+  if(DALI_LIKELY(!mDestroyThread))
+  {
+    mSleepTimePoint = timeToSleepUntil;
+    mNeedToSleep    = true;
+    mConditionalWait.Notify(lock);
+  }
+}
+
+void VectorAnimationThread::SleepThread::Finalize()
+{
+  Mutex::ScopedLock awakeLock(mAwakeCallbackMutex);
+  Mutex::ScopedLock sleepLock(mSleepRequestMutex);
+  if(DALI_LIKELY(!mDestroyThread))
+  {
+    DALI_LOG_DEBUG_INFO("Mark VectorAnimationThread::SleepThread destroyed\n");
+    mDestroyThread = true;
+  }
+  mAwakeCallback.reset();
 }
 
 void VectorAnimationThread::SleepThread::Run()
@@ -437,31 +462,39 @@ void VectorAnimationThread::SleepThread::Run()
 
   while(!mDestroyThread)
   {
-    bool                                               needToSleep;
+    bool needToSleep = false;
+
     std::chrono::time_point<std::chrono::steady_clock> sleepTimePoint;
 
     {
       ConditionalWait::ScopedLock lock(mConditionalWait);
+      Mutex::ScopedLock           sleepLock(mSleepRequestMutex);
 
-      needToSleep    = mNeedToSleep;
-      sleepTimePoint = mSleepTimePoint;
+      if(DALI_LIKELY(!mDestroyThread))
+      {
+        needToSleep    = mNeedToSleep;
+        sleepTimePoint = mSleepTimePoint;
 
-      mNeedToSleep = false;
+        mNeedToSleep = false;
+      }
     }
 
     if(needToSleep)
     {
       std::this_thread::sleep_until(sleepTimePoint);
 
-      if(mAwakeCallback)
       {
-        CallbackBase::Execute(*mAwakeCallback);
+        Mutex::ScopedLock awakeLock(mAwakeCallbackMutex);
+        if(DALI_LIKELY(mAwakeCallback))
+        {
+          CallbackBase::Execute(*mAwakeCallback);
+        }
       }
     }
 
     {
       ConditionalWait::ScopedLock lock(mConditionalWait);
-      if(!mDestroyThread && !mNeedToSleep)
+      if(DALI_LIKELY(!mDestroyThread) && !mNeedToSleep)
       {
         mConditionalWait.Wait(lock);
       }
diff --git a/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.h b/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.h
index ac5053b70d..22d82ce3a2 100644
--- a/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.h
+++ b/dali-toolkit/internal/visuals/animated-vector-image/vector-animation-thread.h
@@ -150,6 +150,11 @@ private:
      */
     void SleepUntil(std::chrono::time_point<std::chrono::steady_clock> timeToSleepUntil);
 
+    /**
+     * @brief Finalizes the sleep thread. This will make ensure that we don't touch VectorAnimationThread.
+     */
+    void Finalize();
+
   protected:
     /**
      * @brief The entry function of the animation thread.
@@ -161,7 +166,10 @@ private:
     SleepThread& operator=(const SleepThread& thread) = delete;
 
   private:
-    ConditionalWait                                    mConditionalWait;
+    ConditionalWait mConditionalWait;
+    Mutex           mAwakeCallbackMutex; ///< Mutex to check validatoin of mAwakeCallback
+    Mutex           mSleepRequestMutex;  ///< Mutex to change sleep time point.
+
     std::unique_ptr<CallbackBase>                      mAwakeCallback;
     std::chrono::time_point<std::chrono::steady_clock> mSleepTimePoint;
     const Dali::LogFactoryInterface&                   mLogFactory;