Improve checking for event/socket notifiers and timers

Starting/stopping timers from another thread may result in errors that
may not appear until hours, days or weeks after if a release build of
Qt is used with the GLib/UNIX event dispatchers. Such errors may
manifest as warnings such as "QObject::killTimer(): Error: timer id 7
is not valid for object 0x2a51b488 (), timer has not been killed" and
application crashes (e.g. crashes in malloc, realloc and
malloc_consolidate).

Initial-patch-by: Eike Ziller <eike.ziller@digia.com>
Task-number: QTBUG-40636
Change-Id: I2de50d50eb1fc7467fcebb9c73b74d2f85137933
Reviewed-by: Jocelyn Turcotte <jocelyn.turcotte@digia.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

1 files changed, 29 insertions, 9 deletions

diff --git a/src/corelib/kernel/qbasictimer.cpp b/src/corelib/kernel/qbasictimer.cpp
index f3366489d5..377c45219c 100644
--- a/src/corelib/kernel/qbasictimer.cpp
+++ b/src/corelib/kernel/qbasictimer.cpp
@@ -118,13 +118,19 @@ QT_BEGIN_NAMESPACE
 void QBasicTimer::start(int msec, QObject *obj)
 {
     QAbstractEventDispatcher *eventDispatcher = QAbstractEventDispatcher::instance();
-    if (!eventDispatcher) {
+    if (Q_UNLIKELY(!eventDispatcher)) {
         qWarning("QBasicTimer::start: QBasicTimer can only be used with threads started with QThread");
         return;
     }
+    if (Q_UNLIKELY(obj && obj->thread() != eventDispatcher->thread())) {
+        qWarning("QBasicTimer::start: Timers cannot be started from another thread");
+        return;
+    }
     if (id) {
-        eventDispatcher->unregisterTimer(id);
-        QAbstractEventDispatcherPrivate::releaseTimerId(id);
+        if (Q_LIKELY(eventDispatcher->unregisterTimer(id)))
+            QAbstractEventDispatcherPrivate::releaseTimerId(id);
+        else
+            qWarning("QBasicTimer::start: Stopping previous timer failed. Possibly trying to stop from a different thread");
     }
     id = 0;
     if (obj)
@@ -145,13 +151,23 @@ void QBasicTimer::start(int msec, QObject *obj)
 void QBasicTimer::start(int msec, Qt::TimerType timerType, QObject *obj)
 {
     QAbstractEventDispatcher *eventDispatcher = QAbstractEventDispatcher::instance();
-    if (!eventDispatcher) {
+    if (Q_UNLIKELY(msec < 0)) {
+        qWarning("QBasicTimer::start: Timers cannot have negative timeouts");
+        return;
+    }
+    if (Q_UNLIKELY(!eventDispatcher)) {
         qWarning("QBasicTimer::start: QBasicTimer can only be used with threads started with QThread");
         return;
     }
+    if (Q_UNLIKELY(obj && obj->thread() != eventDispatcher->thread())) {
+        qWarning("QBasicTimer::start: Timers cannot be started from another thread");
+        return;
+    }
     if (id) {
-        eventDispatcher->unregisterTimer(id);
-        QAbstractEventDispatcherPrivate::releaseTimerId(id);
+        if (Q_LIKELY(eventDispatcher->unregisterTimer(id)))
+            QAbstractEventDispatcherPrivate::releaseTimerId(id);
+        else
+            qWarning("QBasicTimer::start: Stopping previous timer failed. Possibly trying to stop from a different thread");
     }
     id = 0;
     if (obj)
@@ -167,9 +183,13 @@ void QBasicTimer::stop()
 {
     if (id) {
         QAbstractEventDispatcher *eventDispatcher = QAbstractEventDispatcher::instance();
-        if (eventDispatcher)
-            eventDispatcher->unregisterTimer(id);
-        QAbstractEventDispatcherPrivate::releaseTimerId(id);
+        if (eventDispatcher) {
+            if (Q_UNLIKELY(!eventDispatcher->unregisterTimer(id))) {
+                qWarning("QBasicTimer::stop: Failed. Possibly trying to stop from a different thread");
+                return;
+            }
+            QAbstractEventDispatcherPrivate::releaseTimerId(id);
+        }
     }
     id = 0;
 }