浏览代码

Fix dead locks when sendEvent is called from a "handleEvent" method.

Sascha Zelzer 13 年之前
父节点
当前提交
93d2a3f719

+ 21 - 0
Plugins/org.commontk.eventadmin/dispatch/ctkEAInterruptibleThread_p.h

@@ -44,6 +44,27 @@ public:
   void setAutoDelete(bool autoDelete) { ref = autoDelete ? 0 : -1; }
 };
 
+class ctkEAScopedRunnableReference
+{
+
+public:
+
+  ctkEAScopedRunnableReference(ctkEARunnable* runnable)
+    : runnable(runnable)
+  {
+    ++runnable->ref;
+  }
+
+  ~ctkEAScopedRunnableReference()
+  {
+    if (!--runnable->ref) delete runnable;
+  }
+
+private:
+
+  ctkEARunnable* runnable;
+};
+
 /**
  * A QThread subclass which can be interrupted when waiting
  * on a wait condition.

+ 2 - 12
Plugins/org.commontk.eventadmin/tasks/ctkEAHandlerTask.tpp

@@ -61,8 +61,7 @@ private:
 template<class BlacklistingHandlerTasks>
 ctkEAHandlerTask<BlacklistingHandlerTasks>::ctkEAHandlerTask(const ctkServiceReference& eventHandlerRef,
                                                              const ctkEvent& event, BlacklistingHandlerTasks* handlerTasks)
-  : eventHandlerRef(eventHandlerRef), event(event), handlerTasks(handlerTasks),
-    finishedTask(false)
+  : eventHandlerRef(eventHandlerRef), event(event), handlerTasks(handlerTasks)
 {
 
 }
@@ -70,7 +69,7 @@ ctkEAHandlerTask<BlacklistingHandlerTasks>::ctkEAHandlerTask(const ctkServiceRef
 template<class BlacklistingHandlerTasks>
 ctkEAHandlerTask<BlacklistingHandlerTasks>::ctkEAHandlerTask(const Self& task)
   : eventHandlerRef(task.eventHandlerRef), event(task.event),
-    handlerTasks(task.handlerTasks), finishedTask(task.finishedTask)
+    handlerTasks(task.handlerTasks)
 {
 
 }
@@ -82,7 +81,6 @@ ctkEAHandlerTask<BlacklistingHandlerTasks>::operator=(const Self& task)
   eventHandlerRef = task.eventHandlerRef;
   event = task.event;
   handlerTasks = task.handlerTasks;
-  finishedTask = task.finishedTask;
   return *this;
 }
 
@@ -110,7 +108,6 @@ void ctkEAHandlerTask<BlacklistingHandlerTasks>::execute()
         << "Exception during event dispatch [" << event.getTopic() << "| Plugin("
         << eventHandlerRef.getPlugin()->getSymbolicName() << ")]";
   }
-  finishedTask.testAndSetOrdered(0, 1);
 }
 
 template<class BlacklistingHandlerTasks>
@@ -118,10 +115,3 @@ void ctkEAHandlerTask<BlacklistingHandlerTasks>::blackListHandler()
 {
   handlerTasks->blackListRef(eventHandlerRef);
 }
-
-template<class BlacklistingHandlerTasks>
-bool ctkEAHandlerTask<BlacklistingHandlerTasks>::finished() const
-{
-  return finishedTask.fetchAndAddOrdered(0);
-}
-

+ 0 - 8
Plugins/org.commontk.eventadmin/tasks/ctkEAHandlerTask_p.h

@@ -49,9 +49,6 @@ private:
   // Used to blacklist the service or get the service object for the reference
   BlacklistingHandlerTasks* handlerTasks;
 
-  // Is this handler finished
-  mutable QAtomicInt finishedTask;
-
   class _GetAndUngetEventHandler;
 
 public:
@@ -86,11 +83,6 @@ public:
    */
   void blackListHandler();
 
-  /**
-   * Is the delivery finished
-   */
-  bool finished() const;
-
 };
 
 #include "ctkEAHandlerTask.tpp"

+ 27 - 71
Plugins/org.commontk.eventadmin/tasks/ctkEASyncDeliverTasks.tpp

@@ -37,17 +37,14 @@ public:
   ctkEARendezvous timerBarrier;
   ctkEARendezvous startBarrier;
 
-  _TimeoutRunnable(ctkEARendezvous* cascadingBarrier, HandlerTask* task)
-    : cascadingBarrier(cascadingBarrier), task(task)
+  _TimeoutRunnable(HandlerTask* task)
+    : task(task)
   {
 
   }
 
   void run()
   {
-    ctkEASyncThread* myThread = qobject_cast<ctkEASyncThread*>(QThread::currentThread());
-    Q_ASSERT(myThread != 0);
-    myThread->init(&timerBarrier, cascadingBarrier);
     try
     {
       // notify the outer thread to start the timer
@@ -61,17 +58,10 @@ public:
     {
       // this can happen on shutdown, so we ignore it
     }
-    catch (...)
-    {
-      myThread->uninit();
-      throw;
-    }
-    myThread->uninit();
   }
 
 private:
 
-  ctkEARendezvous* cascadingBarrier;
   HandlerTask* task;
 };
 
@@ -152,19 +142,8 @@ void ctkEASyncDeliverTasks<HandlerTask>::execute(const QList<HandlerTask>& tasks
 template<class HandlerTask>
 void ctkEASyncDeliverTasks<HandlerTask>::executeInSyncMaster(const QList<HandlerTask>& tasks)
 {
-  QThread* sleepingThread = QThread::currentThread();
-  ctkEASyncThread* syncThread = qobject_cast<ctkEASyncThread*>(sleepingThread);
-  ctkEARendezvous cascadingBarrier;
-  // check if this is a cascaded event sending
-  if (syncThread)
-  {
-    // wake up outer thread
-    if (syncThread->isTopMostHandler())
-    {
-      syncThread->getTimerBarrier()->waitForRendezvous();
-    }
-    syncThread->innerEventHandlingStart();
-  }
+  QThread* const sleepingThread = QThread::currentThread();
+  ctkEASyncThread* const syncThread = qobject_cast<ctkEASyncThread*>(sleepingThread);
 
   foreach(HandlerTask task, tasks)
   {
@@ -173,11 +152,24 @@ void ctkEASyncDeliverTasks<HandlerTask>::executeInSyncMaster(const QList<Handler
       // no timeout, we can directly execute
       task.execute();
     }
+    else if (syncThread != 0)
+    {
+      // if this is a cascaded event, we directly use this thread
+      // otherwise we could end up in a starvation
+      //TODO use Qt4.7 API
+      //long startTime = System.currentTimeMillis();
+      QDateTime startTime = QDateTime::currentDateTime();
+      task.execute();
+      if (startTime.time().msecsTo(QDateTime::currentDateTime().time()) > timeout)
+      {
+        task.blackListHandler();
+      }
+    }
     else
     {
       _TimeoutRunnable<HandlerTask>* timeoutRunnable
-          = new _TimeoutRunnable<HandlerTask>(&cascadingBarrier, &task);
-      ++timeoutRunnable->ref;
+          = new _TimeoutRunnable<HandlerTask>(&task);
+      ctkEAScopedRunnableReference runnableRef(timeoutRunnable);
 
       ctkEARendezvous* startBarrier = &timeoutRunnable->startBarrier;
       ctkEARendezvous* timerBarrier = &timeoutRunnable->timerBarrier;
@@ -187,55 +179,19 @@ void ctkEASyncDeliverTasks<HandlerTask>::executeInSyncMaster(const QList<Handler
       startBarrier->waitForRendezvous();
 
       // timeout handling
-      bool finished = true;
-      long sleepTime = timeout;
-      do {
-        finished = true;
-        // we sleep for the sleep time
-        // if someone wakes us up it's the inner task who either
-        // has finished or a cascading event
-        //TODO use Qt4.7 API
-        //long startTime = System.currentTimeMillis();
-        QDateTime startTime = QDateTime::currentDateTime();
-        try
-        {
-          timerBarrier->waitAttemptForRendezvous(sleepTime);
-          // if this occurs no timeout occured or we have a cascaded event
-          if (!task.finished())
-          {
-            // adjust remaining sleep time
-            //TODO use Qt4.7 API
-            //sleepTime = timeout - (System.currentTimeMillis() - startTime);
-            sleepTime = timeout - startTime.time().msecsTo(QDateTime::currentDateTime().time());
-            cascadingBarrier.waitForRendezvous();
-            finished = task.finished();
-          }
-        }
-        catch (const ctkEATimeoutException& )
-        {
-          // if we timed out, we have to blacklist the handler
-          task.blackListHandler();
-        }
+      // we sleep for the sleep time
+      // if someone wakes us up it's the finished inner task
+      try
+      {
+        timerBarrier->waitAttemptForRendezvous(timeout);
       }
-      while ( !finished );
-
-      if (!--timeoutRunnable->ref) delete timeoutRunnable;
-    }
-  }
-
-  // wake up outer thread again if cascaded
-  if (syncThread)
-  {
-    syncThread->innerEventHandlingStopped();
-    if (syncThread->isTopMostHandler())
-    {
-      if (!syncThread->getTimerBarrier()->isTimedOut())
+      catch (const ctkEATimeoutException& )
       {
-        syncThread->getCascadingBarrier()->waitForRendezvous();
+        // if we timed out, we have to blacklist the handler
+        task.blackListHandler();
       }
     }
   }
-
 }
 
 template<class HandlerTask>

+ 6 - 0
Plugins/org.commontk.eventadmin/tasks/ctkEASyncDeliverTasks_p.h

@@ -70,11 +70,17 @@ private:
   /** The timeout for event handlers, 0 = disabled. */
   long timeout;
 
+  /**
+   * The matcher interface for checking if timeout handling
+   * is disabled for the handler.
+   * Matching is based on the class name of the event handler.
+   */
   struct Matcher
   {
     virtual bool match(const QString& className) const = 0;
   };
 
+  /** Match a class name. */
   struct ClassMatcher : public Matcher
   {
   private:

+ 1 - 38
Plugins/org.commontk.eventadmin/tasks/ctkEASyncThread.cpp

@@ -23,44 +23,7 @@
 #include "ctkEASyncThread_p.h"
 
 ctkEASyncThread::ctkEASyncThread(ctkEARunnable* target, QObject* parent)
-  : ctkEAInterruptibleThread(target, parent), counter(0)
+  : ctkEAInterruptibleThread(target, parent)
 {
   this->setObjectName(QString("ctkEASyncThread") + QString::number(reinterpret_cast<qint64>(target)));
 }
-
-void ctkEASyncThread::init(ctkEARendezvous* timerBarrier, ctkEARendezvous* cascadingBarrier)
-{
-  this->timerBarrier.testAndSetOrdered(0, timerBarrier);
-  this->cascadingBarrier.testAndSetOrdered(0, cascadingBarrier);
-}
-
-void ctkEASyncThread::uninit()
-{
-  this->timerBarrier.testAndSetOrdered(timerBarrier, 0);
-  this->cascadingBarrier.testAndSetOrdered(cascadingBarrier, 0);
-}
-
-ctkEARendezvous* ctkEASyncThread::getTimerBarrier() const
-{
-  return timerBarrier.fetchAndAddOrdered(0);
-}
-
-ctkEARendezvous* ctkEASyncThread::getCascadingBarrier() const
-{
-  return cascadingBarrier.fetchAndAddOrdered(0);
-}
-
-bool ctkEASyncThread::isTopMostHandler() const
-{
-  return counter.fetchAndAddOrdered(0) == 0;
-}
-
-void ctkEASyncThread::innerEventHandlingStart()
-{
-  counter.ref();
-}
-
-void ctkEASyncThread::innerEventHandlingStopped()
-{
-  counter.deref();
-}

+ 1 - 20
Plugins/org.commontk.eventadmin/tasks/ctkEASyncThread_p.h

@@ -32,22 +32,13 @@
 
 /**
  * This thread class is used for sending the events
- * synchronously.
+ * synchronously. It acts like a marker.
  */
 class ctkEASyncThread : public ctkEAInterruptibleThread
 {
 
   Q_OBJECT
 
-private:
-
-  /** Counter to track the nesting level. */
-  mutable QAtomicInt counter;
-
-  /** The barriers for synchronizing. */
-  mutable QAtomicPointer<ctkEARendezvous> timerBarrier;
-  mutable QAtomicPointer<ctkEARendezvous> cascadingBarrier;
-
 public:
 
   /**
@@ -55,16 +46,6 @@ public:
    */
   ctkEASyncThread(ctkEARunnable* target, QObject* parent = 0);
 
-  void init(ctkEARendezvous* timerBarrier, ctkEARendezvous* cascadingBarrier);
-  void uninit();
-
-  ctkEARendezvous* getTimerBarrier() const;
-  ctkEARendezvous* getCascadingBarrier() const;
-
-  bool isTopMostHandler() const;
-
-  void innerEventHandlingStart();
-  void innerEventHandlingStopped();
 };
 
 #endif // CTKEASYNCTHREAD_P_H

+ 0 - 5
Plugins/org.commontk.eventadmin/util/ctkEARendezvous.cpp

@@ -62,8 +62,3 @@ void ctkEARendezvous::waitAttemptForRendezvous(long timeout)
     throw te;
   }
 }
-
-bool ctkEARendezvous::isTimedOut() const
-{
-  return timedout.fetchAndAddOrdered(0);
-}

+ 0 - 1
Plugins/org.commontk.eventadmin/util/ctkEARendezvous_p.h

@@ -56,7 +56,6 @@ public:
    */
   void waitAttemptForRendezvous(long timeout);
 
-  bool isTimedOut() const;
 };
 
 #endif // CTKEARENDEZVOUS_P_H