Fixed a basic bug in the deferred execution framework: when the execution of a deferred method unqueued another one, the application crashed.

src/tl/tl/tlDeferredExecution.cc

@@ -85,6 +85,12 @@ DeferredMethodScheduler::unqueue (DeferredMethodBase *method)
     }
     m = mm;
   }
+  for (std::list<DeferredMethodBase *>::iterator m = m_executing.begin (); m != m_executing.end (); ++m) {
+    if (*m == method) {
+      m_unqueued.insert (method);
+      break;
+    }
+  }
 }
 
 void 
@@ -102,19 +108,31 @@ DeferredMethodScheduler::do_enable (bool en)
 void
 DeferredMethodScheduler::do_execute ()
 {
-  std::list<DeferredMethodBase *> methods;
-
   m_lock.lock ();
-  methods.swap (m_methods);
+  m_executing.clear ();
+  m_unqueued.clear ();
+  m_executing.swap (m_methods);
   m_scheduled = false;
   m_lock.unlock ();
 
   //  do the execution outside the locked range to avoid deadlocks if the method's execution
   //  schedules another call.
-  for (std::list<DeferredMethodBase *>::iterator m = methods.begin (); m != methods.end (); ++m) {
-    (*m)->m_scheduled = false;
-    (*m)->execute ();
+  for (std::list<DeferredMethodBase *>::iterator m = m_executing.begin (); m != m_executing.end (); ++m) {
+    bool still_valid;
+    m_lock.lock ();
+    //  during execution a method may be unqueued - make sure this is not executed
+    still_valid = (m_unqueued.find (*m) == m_unqueued.end ());
+    m_lock.unlock ();
+    if (still_valid) {
+      (*m)->m_scheduled = false;
+      (*m)->execute ();
+    }
   }
+
+  m_lock.lock ();
+  m_unqueued.clear ();
+  m_executing.clear ();
+  m_lock.unlock ();
 }
 
 }

src/tl/tl/tlDeferredExecution.h

@@ -28,6 +28,7 @@
 #include "tlThreads.h"
 
 #include <list>
+#include <set>
 
 namespace tl
 {
@@ -141,7 +142,8 @@ protected:
 private:
   int m_disabled;
   bool m_scheduled;
-  std::list<DeferredMethodBase *> m_methods;
+  std::list<DeferredMethodBase *> m_methods, m_executing;
+  std::set<DeferredMethodBase *> m_unqueued;
   tl::Mutex m_lock;
 
   void do_enable (bool en);
@@ -263,8 +265,6 @@ public:
    */
   void execute ()
   {
-    //  cancel execution which might be pending
-    cancel ();
     (mp_t->*m_method) ();
   }
 

src/tl/unit_tests/tlDeferredExecutionTests.cc

@@ -186,3 +186,29 @@ TEST(1)
 
 }
 
+static int y_inst = 0;
+
+class Y
+{
+public:
+  Y () : da (this, &Y::a), db (this, &Y::b) { ++y_inst; }
+  ~Y () { --y_inst; }
+
+  void a () { delete this; }
+  void b () { tl_assert(false); }
+
+  tl::DeferredMethod<Y> da, db;
+};
+
+TEST(2)
+{
+  //  execution of a deletes db which must not be executed
+  y_inst = 0;
+  Y *y = new Y ();
+  y->da ();
+  y->db ();
+
+  QCoreApplication::instance ()->processEvents ();
+
+  EXPECT_EQ (y_inst, 0);
+}