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>

9 files changed, 89 insertions, 34 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;
 }
diff --git a/src/corelib/kernel/qeventdispatcher_glib.cpp b/src/corelib/kernel/qeventdispatcher_glib.cpp
index e87e830c39..c4a5fc1ea8 100644
--- a/src/corelib/kernel/qeventdispatcher_glib.cpp
+++ b/src/corelib/kernel/qeventdispatcher_glib.cpp
@@ -518,7 +518,7 @@ void QEventDispatcherGlib::registerTimer(int timerId, int interval, Qt::TimerTyp
         qWarning("QEventDispatcherGlib::registerTimer: invalid arguments");
         return;
     } else if (object->thread() != thread() || thread() != QThread::currentThread()) {
-        qWarning("QObject::startTimer: timers cannot be started from another thread");
+        qWarning("QEventDispatcherGlib::registerTimer: timers cannot be started from another thread");
         return;
     }
 #endif
@@ -534,7 +534,7 @@ bool QEventDispatcherGlib::unregisterTimer(int timerId)
         qWarning("QEventDispatcherGlib::unregisterTimer: invalid argument");
         return false;
     } else if (thread() != QThread::currentThread()) {
-        qWarning("QObject::killTimer: timers cannot be stopped from another thread");
+        qWarning("QEventDispatcherGlib::unregisterTimer: timers cannot be stopped from another thread");
         return false;
     }
 #endif
@@ -550,7 +550,7 @@ bool QEventDispatcherGlib::unregisterTimers(QObject *object)
         qWarning("QEventDispatcherGlib::unregisterTimers: invalid argument");
         return false;
     } else if (object->thread() != thread() || thread() != QThread::currentThread()) {
-        qWarning("QObject::killTimers: timers cannot be stopped from another thread");
+        qWarning("QEventDispatcherGlib::unregisterTimers: timers cannot be stopped from another thread");
         return false;
     }
 #endif
diff --git a/src/corelib/kernel/qeventdispatcher_unix.cpp b/src/corelib/kernel/qeventdispatcher_unix.cpp
index 69363bc3c9..9674fd5417 100644
--- a/src/corelib/kernel/qeventdispatcher_unix.cpp
+++ b/src/corelib/kernel/qeventdispatcher_unix.cpp
@@ -338,7 +338,7 @@ void QEventDispatcherUNIX::registerTimer(int timerId, int interval, Qt::TimerTyp
         qWarning("QEventDispatcherUNIX::registerTimer: invalid arguments");
         return;
     } else if (obj->thread() != thread() || thread() != QThread::currentThread()) {
-        qWarning("QObject::startTimer: timers cannot be started from another thread");
+        qWarning("QEventDispatcherUNIX::registerTimer: timers cannot be started from another thread");
         return;
     }
 #endif
@@ -357,7 +357,7 @@ bool QEventDispatcherUNIX::unregisterTimer(int timerId)
         qWarning("QEventDispatcherUNIX::unregisterTimer: invalid argument");
         return false;
     } else if (thread() != QThread::currentThread()) {
-        qWarning("QObject::killTimer: timers cannot be stopped from another thread");
+        qWarning("QEventDispatcherUNIX::unregisterTimer: timers cannot be stopped from another thread");
         return false;
     }
 #endif
@@ -376,7 +376,7 @@ bool QEventDispatcherUNIX::unregisterTimers(QObject *object)
         qWarning("QEventDispatcherUNIX::unregisterTimers: invalid argument");
         return false;
     } else if (object->thread() != thread() || thread() != QThread::currentThread()) {
-        qWarning("QObject::killTimers: timers cannot be stopped from another thread");
+        qWarning("QEventDispatcherUNIX::unregisterTimers: timers cannot be stopped from another thread");
         return false;
     }
 #endif
diff --git a/src/corelib/kernel/qeventdispatcher_win.cpp b/src/corelib/kernel/qeventdispatcher_win.cpp
index f38ac7bf26..b7594d1dd3 100644
--- a/src/corelib/kernel/qeventdispatcher_win.cpp
+++ b/src/corelib/kernel/qeventdispatcher_win.cpp
@@ -908,13 +908,15 @@ void QEventDispatcherWin32::unregisterSocketNotifier(QSocketNotifier *notifier)
 
 void QEventDispatcherWin32::registerTimer(int timerId, int interval, Qt::TimerType timerType, QObject *object)
 {
+#ifndef QT_NO_DEBUG
     if (timerId < 1 || interval < 0 || !object) {
         qWarning("QEventDispatcherWin32::registerTimer: invalid arguments");
         return;
     } else if (object->thread() != thread() || thread() != QThread::currentThread()) {
-        qWarning("QObject::startTimer: timers cannot be started from another thread");
+        qWarning("QEventDispatcherWin32::registerTimer: timers cannot be started from another thread");
         return;
     }
+#endif
 
     Q_D(QEventDispatcherWin32);
 
@@ -936,15 +938,17 @@ void QEventDispatcherWin32::registerTimer(int timerId, int interval, Qt::TimerTy
 
 bool QEventDispatcherWin32::unregisterTimer(int timerId)
 {
+#ifndef QT_NO_DEBUG
     if (timerId < 1) {
         qWarning("QEventDispatcherWin32::unregisterTimer: invalid argument");
         return false;
     }
     QThread *currentThread = QThread::currentThread();
     if (thread() != currentThread) {
-        qWarning("QObject::killTimer: timers cannot be stopped from another thread");
+        qWarning("QEventDispatcherWin32::unregisterTimer: timers cannot be stopped from another thread");
         return false;
     }
+#endif
 
     Q_D(QEventDispatcherWin32);
     if (d->timerVec.isEmpty() || timerId <= 0)
@@ -962,15 +966,17 @@ bool QEventDispatcherWin32::unregisterTimer(int timerId)
 
 bool QEventDispatcherWin32::unregisterTimers(QObject *object)
 {
+#ifndef QT_NO_DEBUG
     if (!object) {
         qWarning("QEventDispatcherWin32::unregisterTimers: invalid argument");
         return false;
     }
     QThread *currentThread = QThread::currentThread();
     if (object->thread() != thread() || thread() != currentThread) {
-        qWarning("QObject::killTimers: timers cannot be stopped from another thread");
+        qWarning("QEventDispatcherWin32::unregisterTimers: timers cannot be stopped from another thread");
         return false;
     }
+#endif
 
     Q_D(QEventDispatcherWin32);
     if (d->timerVec.isEmpty())
diff --git a/src/corelib/kernel/qeventdispatcher_winrt.cpp b/src/corelib/kernel/qeventdispatcher_winrt.cpp
index aec0981677..d75d30f48e 100644
--- a/src/corelib/kernel/qeventdispatcher_winrt.cpp
+++ b/src/corelib/kernel/qeventdispatcher_winrt.cpp
@@ -254,13 +254,15 @@ void QEventDispatcherWinRT::registerTimer(int timerId, int interval, Qt::TimerTy
 {
     Q_UNUSED(timerType);
 
+#ifndef QT_NO_DEBUG
     if (timerId < 1 || interval < 0 || !object) {
         qWarning("QEventDispatcherWinRT::registerTimer: invalid arguments");
         return;
     } else if (object->thread() != thread() || thread() != QThread::currentThread()) {
-        qWarning("QObject::startTimer: timers cannot be started from another thread");
+        qWarning("QEventDispatcherWinRT::registerTimer: timers cannot be started from another thread");
         return;
     }
+#endif
 
     Q_D(QEventDispatcherWinRT);
 
@@ -289,14 +291,16 @@ void QEventDispatcherWinRT::registerTimer(int timerId, int interval, Qt::TimerTy
 
 bool QEventDispatcherWinRT::unregisterTimer(int timerId)
 {
+#ifndef QT_NO_DEBUG
     if (timerId < 1) {
         qWarning("QEventDispatcherWinRT::unregisterTimer: invalid argument");
         return false;
     }
     if (thread() != QThread::currentThread()) {
-        qWarning("QObject::killTimer: timers cannot be stopped from another thread");
+        qWarning("QEventDispatcherWinRT::unregisterTimer: timers cannot be stopped from another thread");
         return false;
     }
+#endif
 
     Q_D(QEventDispatcherWinRT);
 
@@ -310,15 +314,17 @@ bool QEventDispatcherWinRT::unregisterTimer(int timerId)
 
 bool QEventDispatcherWinRT::unregisterTimers(QObject *object)
 {
+#ifndef QT_NO_DEBUG
     if (!object) {
         qWarning("QEventDispatcherWinRT::unregisterTimers: invalid argument");
         return false;
     }
     QThread *currentThread = QThread::currentThread();
     if (object->thread() != thread() || thread() != currentThread) {
-        qWarning("QObject::killTimers: timers cannot be stopped from another thread");
+        qWarning("QEventDispatcherWinRT::unregisterTimers: timers cannot be stopped from another thread");
         return false;
     }
+#endif
 
     Q_D(QEventDispatcherWinRT);
     for (QHash<int, WinRTTimerInfo *>::iterator it = d->timerDict.begin(); it != d->timerDict.end();) {
diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index 45bf4b62c5..bf814f36b9 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -225,13 +225,17 @@ QObjectPrivate::QObjectPrivate(int version)
 QObjectPrivate::~QObjectPrivate()
 {
     if (extraData && !extraData->runningTimers.isEmpty()) {
-        // unregister pending timers
-        if (threadData->eventDispatcher.load())
-            threadData->eventDispatcher.load()->unregisterTimers(q_ptr);
-
-        // release the timer ids back to the pool
-        for (int i = 0; i < extraData->runningTimers.size(); ++i)
-            QAbstractEventDispatcherPrivate::releaseTimerId(extraData->runningTimers.at(i));
+        if (Q_LIKELY(threadData->thread == QThread::currentThread())) {
+            // unregister pending timers
+            if (threadData->eventDispatcher.load())
+                threadData->eventDispatcher.load()->unregisterTimers(q_ptr);
+
+            // release the timer ids back to the pool
+            for (int i = 0; i < extraData->runningTimers.size(); ++i)
+                QAbstractEventDispatcherPrivate::releaseTimerId(extraData->runningTimers.at(i));
+        } else {
+            qWarning("QObject::~QObject: Timers cannot be stopped from another thread");
+        }
     }
 
     if (postedEvents)
@@ -1612,15 +1616,18 @@ int QObject::startTimer(int interval, Qt::TimerType timerType)
 {
     Q_D(QObject);
 
-    if (interval < 0) {
+    if (Q_UNLIKELY(interval < 0)) {
         qWarning("QObject::startTimer: Timers cannot have negative intervals");
         return 0;
     }
-
-    if (!d->threadData->eventDispatcher.load()) {
+    if (Q_UNLIKELY(!d->threadData->eventDispatcher.load())) {
         qWarning("QObject::startTimer: Timers can only be used with threads started with QThread");
         return 0;
     }
+    if (Q_UNLIKELY(thread() != QThread::currentThread())) {
+        qWarning("QObject::startTimer: Timers cannot be started from another thread");
+        return 0;
+    }
     int timerId = d->threadData->eventDispatcher.load()->registerTimer(interval, timerType, this);
     if (!d->extraData)
         d->extraData = new QObjectPrivate::ExtraData;
@@ -1640,6 +1647,10 @@ int QObject::startTimer(int interval, Qt::TimerType timerType)
 void QObject::killTimer(int id)
 {
     Q_D(QObject);
+    if (Q_UNLIKELY(thread() != QThread::currentThread())) {
+        qWarning("QObject::killTimer: Timers cannot be stopped from another thread");
+        return;
+    }
     if (id) {
         int at = d->extraData ? d->extraData->runningTimers.indexOf(id) : -1;
         if (at == -1) {
diff --git a/src/corelib/kernel/qsocketnotifier.cpp b/src/corelib/kernel/qsocketnotifier.cpp
index 6470873d9a..2598f02a19 100644
--- a/src/corelib/kernel/qsocketnotifier.cpp
+++ b/src/corelib/kernel/qsocketnotifier.cpp
@@ -274,6 +274,10 @@ void QSocketNotifier::setEnabled(bool enable)
 
     if (!d->threadData->eventDispatcher.load()) // perhaps application/thread is shutting down
         return;
+    if (Q_UNLIKELY(thread() != QThread::currentThread())) {
+        qWarning("QSocketNotifier: Socket notifiers cannot be enabled or disabled from another thread");
+        return;
+    }
     if (d->snenabled)
         d->threadData->eventDispatcher.load()->registerSocketNotifier(this);
     else
diff --git a/src/corelib/kernel/qtimer.cpp b/src/corelib/kernel/qtimer.cpp
index 97aae6f7e0..5c09b483c6 100644
--- a/src/corelib/kernel/qtimer.cpp
+++ b/src/corelib/kernel/qtimer.cpp
@@ -395,6 +395,10 @@ void QTimer::singleShot(int msec, const QObject *receiver, const char *member)
 */
 void QTimer::singleShot(int msec, Qt::TimerType timerType, const QObject *receiver, const char *member)
 {
+    if (Q_UNLIKELY(msec < 0)) {
+        qWarning("QTimer::singleShot: Timers cannot have negative timeouts");
+        return;
+    }
     if (receiver && member) {
         if (msec == 0) {
             // special code shortpath for 0-timers
diff --git a/src/corelib/kernel/qwineventnotifier.cpp b/src/corelib/kernel/qwineventnotifier.cpp
index c694237dc3..d2c623d489 100644
--- a/src/corelib/kernel/qwineventnotifier.cpp
+++ b/src/corelib/kernel/qwineventnotifier.cpp
@@ -140,11 +140,11 @@ QWinEventNotifier::QWinEventNotifier(HANDLE hEvent, QObject *parent)
 {
     Q_D(QWinEventNotifier);
     QAbstractEventDispatcher *eventDispatcher = d->threadData->eventDispatcher.load();
-    if (!eventDispatcher) {
+    if (Q_UNLIKELY(!eventDispatcher)) {
         qWarning("QWinEventNotifier: Can only be used with threads started with QThread");
-    } else {
-        eventDispatcher->registerEventNotifier(this);
+        return;
     }
+    eventDispatcher->registerEventNotifier(this);
     d->enabled = true;
 }
 
@@ -215,6 +215,10 @@ void QWinEventNotifier::setEnabled(bool enable)
     QAbstractEventDispatcher *eventDispatcher = d->threadData->eventDispatcher.load();
     if (!eventDispatcher) // perhaps application is shutting down
         return;
+    if (Q_UNLIKELY(thread() != QThread::currentThread())) {
+        qWarning("QWinEventNotifier: Event notifiers cannot be enabled or disabled from another thread");
+        return;
+    }
 
     if (enable)
         eventDispatcher->registerEventNotifier(this);