From d39d7542f3a45ada3907506a6880053634e35274 Mon Sep 17 00:00:00 2001
From: vng <viktor.govako@gmail.com>
Date: Fri, 5 Jul 2013 15:44:03 +0300
Subject: [PATCH] - Do non-blocking Cancel in ScheduledTask. Blocking Cancel
 "causes" deadlock and freezing of main thread. - Fix memory leak.

---
 android/jni/com/mapswithme/maps/Framework.cpp | 15 +++++++----
 android/jni/com/mapswithme/maps/Framework.hpp |  2 +-
 base/scheduled_task.cpp                       | 26 +++++++++++--------
 base/scheduled_task.hpp                       | 25 ++++++++----------
 qt/draw_widget.cpp                            | 18 +++++++++----
 qt/draw_widget.hpp                            |  2 +-
 6 files changed, 51 insertions(+), 37 deletions(-)

diff --git a/android/jni/com/mapswithme/maps/Framework.cpp b/android/jni/com/mapswithme/maps/Framework.cpp
index 6d5a0b9b6f..b02aed6c54 100644
--- a/android/jni/com/mapswithme/maps/Framework.cpp
+++ b/android/jni/com/mapswithme/maps/Framework.cpp
@@ -246,18 +246,23 @@ namespace android
 
   void Framework::StartTouchTask(double x, double y, unsigned ms)
   {
-    KillTouchTask();
-
-    m_scheduledTask.reset(new ScheduledTask(bind(&android::Framework::OnProcessTouchTask, this, x, y, ms), ms));
+    if (KillTouchTask())
+      m_scheduledTask.reset(new ScheduledTask(bind(&android::Framework::OnProcessTouchTask, this, x, y, ms), ms));
   }
 
-  void Framework::KillTouchTask()
+  bool Framework::KillTouchTask()
   {
     if (m_scheduledTask)
     {
-      m_scheduledTask->Cancel();
+      if (!m_scheduledTask->Cancel())
+      {
+        // The task is already running - skip new task.
+        return false;
+      }
+
       m_scheduledTask.reset();
     }
+    return true;
   }
 
   /// @param[in] mask Active pointers bits : 0x0 - no, 0x1 - (x1, y1), 0x2 - (x2, y2), 0x3 - (x1, y1)(x2, y2).
diff --git a/android/jni/com/mapswithme/maps/Framework.hpp b/android/jni/com/mapswithme/maps/Framework.hpp
index b9c5ee03ee..a668109017 100644
--- a/android/jni/com/mapswithme/maps/Framework.hpp
+++ b/android/jni/com/mapswithme/maps/Framework.hpp
@@ -52,7 +52,7 @@ namespace android
     bool m_wasLongClick;
 
     void StartTouchTask(double x, double y, unsigned ms);
-    void KillTouchTask();
+    bool KillTouchTask();
     void OnProcessTouchTask(double x, double y, unsigned ms);
 
     string m_searchQuery;
diff --git a/base/scheduled_task.cpp b/base/scheduled_task.cpp
index 6aebefdd7c..e96f9ad01e 100644
--- a/base/scheduled_task.cpp
+++ b/base/scheduled_task.cpp
@@ -31,20 +31,24 @@ void ScheduledTask::Routine::Do()
   m_pCond->Unlock();
 }
 
-void ScheduledTask::Routine::Cancel()
-{
-  m_pCond->Lock();
-  IRoutine::Cancel();
-  m_pCond->Signal();
-  m_pCond->Unlock();
-}
-
 ScheduledTask::ScheduledTask(fn_t const & fn, unsigned ms)
 {
-  m_thread.Create(new Routine(fn, ms, &m_cond));
+  m_routine.reset(new Routine(fn, ms, &m_cond));
+  m_thread.Create(m_routine.get());
 }
 
-void ScheduledTask::Cancel()
+bool ScheduledTask::Cancel()
 {
-  m_thread.Cancel();
+  if (m_cond.TryLock())
+  {
+    m_routine->Cancel();
+
+    m_cond.Signal();
+    m_cond.Unlock();
+
+    m_thread.Join();
+    return true;
+  }
+
+  return false;
 }
diff --git a/base/scheduled_task.hpp b/base/scheduled_task.hpp
index f8eed92611..86a885c1a1 100644
--- a/base/scheduled_task.hpp
+++ b/base/scheduled_task.hpp
@@ -1,41 +1,38 @@
 #pragma once
 
-#include "../std/function.hpp"
-
 #include "thread.hpp"
 #include "condition.hpp"
 
+#include "../std/function.hpp"
+#include "../std/scoped_ptr.hpp"
+
+
 /// Class, which performs any function when the specified
 /// amount of time is elapsed.
 class ScheduledTask
 {
-public:
-
   typedef function<void()> fn_t;
 
   class Routine : public threads::IRoutine
   {
-  private:
     fn_t m_fn;
     unsigned m_ms;
     threads::Condition * m_pCond;
+
   public:
-
     Routine(fn_t const & fn, unsigned ms, threads::Condition * cond);
-
-    void Do();
-    void Cancel();
+    virtual void Do();
   };
 
-private:
-
+  scoped_ptr<Routine> m_routine;
   threads::Thread m_thread;
   threads::Condition m_cond;
 
 public:
-
   /// Constructor by function and time in miliseconds.
   ScheduledTask(fn_t const & fn, unsigned ms);
-  /// Task could be cancelled before time elapses.
-  void Cancel();
+
+  /// Task could be cancelled before time elapses. This function is NON-blocking.
+  /// @return false If the task is already running.
+  bool Cancel();
 };
diff --git a/qt/draw_widget.cpp b/qt/draw_widget.cpp
index 1cbe7a941f..42c1260f2c 100644
--- a/qt/draw_widget.cpp
+++ b/qt/draw_widget.cpp
@@ -100,9 +100,12 @@ namespace qt
   void DrawWidget::PrepareShutdown()
   {
     KillPressTask();
+
     ASSERT(isValid(), ());
     makeCurrent();
+
     m_framework->PrepareToShutdown();
+
     m_videoTimer.reset();
   }
 
@@ -332,18 +335,23 @@ namespace qt
 
   void DrawWidget::StartPressTask(m2::PointD const & pt, unsigned ms)
   {
-    KillPressTask();
-
-    m_scheduledTasks.reset(new ScheduledTask(bind(&DrawWidget::OnPressTaskEvent, this, pt, ms), ms));
+    if (KillPressTask())
+      m_scheduledTasks.reset(new ScheduledTask(bind(&DrawWidget::OnPressTaskEvent, this, pt, ms), ms));
   }
 
-  void DrawWidget::KillPressTask()
+  bool DrawWidget::KillPressTask()
   {
     if (m_scheduledTasks)
     {
-      m_scheduledTasks->Cancel();
+      if (!m_scheduledTasks->Cancel())
+      {
+        // The task is already running - skip new task.
+        return false;
+      }
+
       m_scheduledTasks.reset();
     }
+    return true;
   }
 
   void DrawWidget::OnPressTaskEvent(m2::PointD const & pt, unsigned ms)
diff --git a/qt/draw_widget.hpp b/qt/draw_widget.hpp
index 760274c948..ec3f1f3266 100644
--- a/qt/draw_widget.hpp
+++ b/qt/draw_widget.hpp
@@ -111,7 +111,7 @@ namespace qt
 
   protected:
     void StartPressTask(m2::PointD const & pt, unsigned ms);
-    void KillPressTask();
+    bool KillPressTask();
     void OnPressTaskEvent(m2::PointD const & pt, unsigned ms);
 
   protected: