Fix invalid memory write during recursive timer activation

The handler for one timer recurses the event loop, and the handler for
another timer removes the first timer, returning from the recursion and
the handler for the first timer causes an invalid write (since the timer
info for the first timer has been deleted).

Fix this by keeping an active reference in QTimerInfo (instead of just a
bool inTimerEvent). If this is non-zero, the timer is currently being
delivered, so we prevent more delivery. When a timer is removed and it's
activateRef is set, we clear it so that the delivery code knows now to
write to memory that's already been freed.

A side effect of this change is that we no longer need to track the
currentTimerInfo "globally" anymore, it can be a normal local variable
in the QTimerInfoList::activateTimers() function.

Task-number: QT-3553
Reviewed-by: olivier
Reviewed-by: joao

2 files changed, 15 insertions, 20 deletions

diff --git a/src/corelib/kernel/qeventdispatcher_unix.cpp b/src/corelib/kernel/qeventdispatcher_unix.cpp
index f7d45ac..9dadd82 100644
--- a/src/corelib/kernel/qeventdispatcher_unix.cpp
+++ b/src/corelib/kernel/qeventdispatcher_unix.cpp
@@ -331,7 +331,7 @@ QTimerInfoList::QTimerInfoList()
     }
 #endif
 
-    firstTimerInfo = currentTimerInfo = 0;
+    firstTimerInfo = 0;
 }
 
 timeval QTimerInfoList::updateCurrentTime()
@@ -445,7 +445,7 @@ bool QTimerInfoList::timerWait(timeval &tm)
     // Find first waiting timer not already active
     QTimerInfo *t = 0;
     for (QTimerInfoList::const_iterator it = constBegin(); it != constEnd(); ++it) {
-        if (!(*it)->inTimerEvent) {
+        if (!(*it)->activateRef) {
             t = *it;
             break;
         }
@@ -474,7 +474,7 @@ void QTimerInfoList::registerTimer(int timerId, int interval, QObject *object)
     t->interval.tv_usec = (interval % 1000) * 1000;
     t->timeout = updateCurrentTime() + t->interval;
     t->obj = object;
-    t->inTimerEvent = false;
+    t->activateRef = 0;
 
     timerInsert(t);
 }
@@ -489,8 +489,8 @@ bool QTimerInfoList::unregisterTimer(int timerId)
             removeAt(i);
             if (t == firstTimerInfo)
                 firstTimerInfo = 0;
-            if (t == currentTimerInfo)
-                currentTimerInfo = 0;
+            if (t->activateRef)
+                *(t->activateRef) = 0;
 
             // release the timer id
             if (!QObjectPrivate::get(t->obj)->inThreadChangeEvent)
@@ -515,8 +515,8 @@ bool QTimerInfoList::unregisterTimers(QObject *object)
             removeAt(i);
             if (t == firstTimerInfo)
                 firstTimerInfo = 0;
-            if (t == currentTimerInfo)
-                currentTimerInfo = 0;
+            if (t->activateRef)
+                *(t->activateRef) = 0;
 
             // release the timer id
             if (!QObjectPrivate::get(t->obj)->inThreadChangeEvent)
@@ -552,10 +552,7 @@ int QTimerInfoList::activateTimers()
     bool firstTime = true;
     timeval currentTime;
     int n_act = 0, maxCount = count();
-
-    QTimerInfo *saveFirstTimerInfo = firstTimerInfo;
-    QTimerInfo *saveCurrentTimerInfo = currentTimerInfo;
-    firstTimerInfo = currentTimerInfo = 0;
+    firstTimerInfo = 0;
 
     while (maxCount--) {
         currentTime = updateCurrentTime();
@@ -567,7 +564,7 @@ int QTimerInfoList::activateTimers()
         if (isEmpty())
             break;
 
-        currentTimerInfo = first();
+        QTimerInfo *currentTimerInfo = first();
         if (currentTime < currentTimerInfo->timeout)
             break; // no timer has expired
 
@@ -594,21 +591,19 @@ int QTimerInfoList::activateTimers()
         if (currentTimerInfo->interval.tv_usec > 0 || currentTimerInfo->interval.tv_sec > 0)
             n_act++;
 
-        if (!currentTimerInfo->inTimerEvent) {
+        if (!currentTimerInfo->activateRef) {
             // send event, but don't allow it to recurse
-            currentTimerInfo->inTimerEvent = true;
+            currentTimerInfo->activateRef = &currentTimerInfo;
 
             QTimerEvent e(currentTimerInfo->id);
             QCoreApplication::sendEvent(currentTimerInfo->obj, &e);
 
             if (currentTimerInfo)
-                currentTimerInfo->inTimerEvent = false;
+                currentTimerInfo->activateRef = 0;
         }
     }
 
-    firstTimerInfo = saveFirstTimerInfo;
-    currentTimerInfo = saveCurrentTimerInfo;
-
+    firstTimerInfo = 0;
     return n_act;
 }
 
diff --git a/src/corelib/kernel/qeventdispatcher_unix_p.h b/src/corelib/kernel/qeventdispatcher_unix_p.h
index cbe58de..060a163 100644
--- a/src/corelib/kernel/qeventdispatcher_unix_p.h
+++ b/src/corelib/kernel/qeventdispatcher_unix_p.h
@@ -77,7 +77,7 @@ struct QTimerInfo {
     timeval interval; // - timer interval
     timeval timeout;  // - when to sent event
     QObject *obj;     // - object to receive event
-    bool inTimerEvent;
+    QTimerInfo **activateRef; // - ref from activateTimers
 };
 
 class QTimerInfoList : public QList<QTimerInfo*>
@@ -92,7 +92,7 @@ class QTimerInfoList : public QList<QTimerInfo*>
 #endif
 
     // state variables used by activateTimers()
-    QTimerInfo *firstTimerInfo, *currentTimerInfo;
+    QTimerInfo *firstTimerInfo;
 
 public:
     QTimerInfoList();