QThread: fix race when setting the eventDispatcher

Use QAtomicPointer to make this thread-safe.

Change-Id: If71f204699fcefabdb59bd26342d777d1cc9e2a7
Reviewed-by: Olivier Goffart <ogoffart@woboq.com>

13 files changed, 58 insertions, 54 deletions

diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 997848851d..7f7066271b 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -380,7 +380,7 @@ bool QProcessPrivate::createChannel(Channel &channel)
             return false;
 
         // create the socket notifiers
-        if (threadData->eventDispatcher) {
+        if (threadData->hasEventDispatcher()) {
             if (&channel == &stdinChannel) {
                 channel.notifier = new QSocketNotifier(channel.pipe[1],
                                                        QSocketNotifier::Write, q);
@@ -562,7 +562,7 @@ void QProcessPrivate::startProcess()
         return;
     }
 
-    if (threadData->eventDispatcher) {
+    if (threadData->hasEventDispatcher()) {
         startupSocketNotifier = new QSocketNotifier(childStartedPipe[0],
                                                     QSocketNotifier::Read, q);
         QObject::connect(startupSocketNotifier, SIGNAL(activated(int)),
diff --git a/src/corelib/io/qprocess_win.cpp b/src/corelib/io/qprocess_win.cpp
index ab1f05777a..0f36c3adbf 100644
--- a/src/corelib/io/qprocess_win.cpp
+++ b/src/corelib/io/qprocess_win.cpp
@@ -528,7 +528,7 @@ void QProcessPrivate::startProcess()
     if (!pid)
         return;
 
-    if (threadData->eventDispatcher) {
+    if (threadData->hasEventDispatcher()) {
         processFinishedNotifier = new QWinEventNotifier(pid->hProcess, q);
         QObject::connect(processFinishedNotifier, SIGNAL(activated(HANDLE)), q, SLOT(_q_processDied()));
         processFinishedNotifier->setEnabled(true);
diff --git a/src/corelib/io/qprocess_wince.cpp b/src/corelib/io/qprocess_wince.cpp
index 4d3f550db5..712748aa59 100644
--- a/src/corelib/io/qprocess_wince.cpp
+++ b/src/corelib/io/qprocess_wince.cpp
@@ -158,7 +158,7 @@ void QProcessPrivate::startProcess()
     if (!pid)
         return;
 
-    if (threadData->eventDispatcher) {
+    if (threadData->hasEventDispatcher()) {
         processFinishedNotifier = new QWinEventNotifier(pid->hProcess, q);
         QObject::connect(processFinishedNotifier, SIGNAL(activated(HANDLE)), q, SLOT(_q_processDied()));
         processFinishedNotifier->setEnabled(true);
diff --git a/src/corelib/kernel/qabstracteventdispatcher.cpp b/src/corelib/kernel/qabstracteventdispatcher.cpp
index c4e02c0347..6558893036 100644
--- a/src/corelib/kernel/qabstracteventdispatcher.cpp
+++ b/src/corelib/kernel/qabstracteventdispatcher.cpp
@@ -172,7 +172,7 @@ QAbstractEventDispatcher::~QAbstractEventDispatcher()
 QAbstractEventDispatcher *QAbstractEventDispatcher::instance(QThread *thread)
 {
     QThreadData *data = thread ? QThreadData::get2(thread) : QThreadData::current();
-    return data->eventDispatcher;
+    return data->eventDispatcher.load();
 }
 
 /*!
diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp
index cce8c30d81..61a8b6a18c 100644
--- a/src/corelib/kernel/qcoreapplication.cpp
+++ b/src/corelib/kernel/qcoreapplication.cpp
@@ -689,7 +689,7 @@ void QCoreApplication::init()
 #ifndef QT_NO_QOBJECT
     // use the event dispatcher created by the app programmer (if any)
     if (!QCoreApplicationPrivate::eventDispatcher)
-        QCoreApplicationPrivate::eventDispatcher = d->threadData->eventDispatcher;
+        QCoreApplicationPrivate::eventDispatcher = d->threadData->eventDispatcher.load();
     // otherwise we create one
     if (!QCoreApplicationPrivate::eventDispatcher)
         d->createEventDispatcher();
@@ -1031,9 +1031,9 @@ bool QCoreApplication::closingDown()
 void QCoreApplication::processEvents(QEventLoop::ProcessEventsFlags flags)
 {
     QThreadData *data = QThreadData::current();
-    if (!data->eventDispatcher)
+    if (!data->hasEventDispatcher())
         return;
-    data->eventDispatcher->processEvents(flags);
+    data->eventDispatcher.load()->processEvents(flags);
 }
 
 /*!
@@ -1055,11 +1055,11 @@ void QCoreApplication::processEvents(QEventLoop::ProcessEventsFlags flags)
 void QCoreApplication::processEvents(QEventLoop::ProcessEventsFlags flags, int maxtime)
 {
     QThreadData *data = QThreadData::current();
-    if (!data->eventDispatcher)
+    if (!data->hasEventDispatcher())
         return;
     QElapsedTimer start;
     start.start();
-    while (data->eventDispatcher->processEvents(flags & ~QEventLoop::WaitForMoreEvents)) {
+    while (data->eventDispatcher.load()->processEvents(flags & ~QEventLoop::WaitForMoreEvents)) {
         if (start.elapsed() > maxtime)
             break;
     }
@@ -1258,8 +1258,9 @@ void QCoreApplication::postEvent(QObject *receiver, QEvent *event, int priority)
     data->canWait = false;
     locker.unlock();
 
-    if (data->eventDispatcher)
-        data->eventDispatcher->wakeUp();
+    QAbstractEventDispatcher* dispatcher = data->eventDispatcher.loadAcquire();
+    if (dispatcher)
+        dispatcher->wakeUp();
 }
 
 /*!
@@ -1379,8 +1380,8 @@ void QCoreApplicationPrivate::sendPostedEvents(QObject *receiver, int event_type
             }
 
             --data->postEventList.recursion;
-            if (!data->postEventList.recursion && !data->canWait && data->eventDispatcher)
-                data->eventDispatcher->wakeUp();
+            if (!data->postEventList.recursion && !data->canWait && data->hasEventDispatcher())
+                data->eventDispatcher.load()->wakeUp();
 
             // clear the global list, i.e. remove everything that was
             // delivered.
diff --git a/src/corelib/kernel/qeventloop.cpp b/src/corelib/kernel/qeventloop.cpp
index fe254b5593..549b8db9ca 100644
--- a/src/corelib/kernel/qeventloop.cpp
+++ b/src/corelib/kernel/qeventloop.cpp
@@ -103,7 +103,7 @@ QEventLoop::QEventLoop(QObject *parent)
     Q_D(QEventLoop);
     if (!QCoreApplication::instance()) {
         qWarning("QEventLoop: Cannot be used without QApplication");
-    } else if (!d->threadData->eventDispatcher) {
+    } else if (!d->threadData->eventDispatcher.load()) {
         QThreadPrivate::createEventDispatcher(d->threadData);
     }
 }
@@ -131,9 +131,9 @@ QEventLoop::~QEventLoop()
 bool QEventLoop::processEvents(ProcessEventsFlags flags)
 {
     Q_D(QEventLoop);
-    if (!d->threadData->eventDispatcher)
+    if (!d->threadData->eventDispatcher.load())
         return false;
-    return d->threadData->eventDispatcher->processEvents(flags);
+    return d->threadData->eventDispatcher.load()->processEvents(flags);
 }
 
 /*!
@@ -234,7 +234,7 @@ int QEventLoop::exec(ProcessEventsFlags flags)
 void QEventLoop::processEvents(ProcessEventsFlags flags, int maxTime)
 {
     Q_D(QEventLoop);
-    if (!d->threadData->eventDispatcher)
+    if (!d->threadData->eventDispatcher.load())
         return;
 
     QElapsedTimer start;
@@ -263,12 +263,12 @@ void QEventLoop::processEvents(ProcessEventsFlags flags, int maxTime)
 void QEventLoop::exit(int returnCode)
 {
     Q_D(QEventLoop);
-    if (!d->threadData->eventDispatcher)
+    if (!d->threadData->eventDispatcher.load())
         return;
 
     d->returnCode = returnCode;
     d->exit = true;
-    d->threadData->eventDispatcher->interrupt();
+    d->threadData->eventDispatcher.load()->interrupt();
 }
 
 /*!
@@ -292,9 +292,9 @@ bool QEventLoop::isRunning() const
 void QEventLoop::wakeUp()
 {
     Q_D(QEventLoop);
-    if (!d->threadData->eventDispatcher)
+    if (!d->threadData->eventDispatcher.load())
         return;
-    d->threadData->eventDispatcher->wakeUp();
+    d->threadData->eventDispatcher.load()->wakeUp();
 }
 
 
diff --git a/src/corelib/kernel/qobject.cpp b/src/corelib/kernel/qobject.cpp
index 1312e64d8b..583e580762 100644
--- a/src/corelib/kernel/qobject.cpp
+++ b/src/corelib/kernel/qobject.cpp
@@ -216,8 +216,8 @@ QObjectPrivate::~QObjectPrivate()
 {
     if (extraData && !extraData->runningTimers.isEmpty()) {
         // unregister pending timers
-        if (threadData->eventDispatcher)
-            threadData->eventDispatcher->unregisterTimers(q_ptr);
+        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)
@@ -1074,7 +1074,7 @@ bool QObject::event(QEvent *e)
     case QEvent::ThreadChange: {
         Q_D(QObject);
         QThreadData *threadData = d->threadData;
-        QAbstractEventDispatcher *eventDispatcher = threadData->eventDispatcher;
+        QAbstractEventDispatcher *eventDispatcher = threadData->eventDispatcher.load();
         if (eventDispatcher) {
             QList<QAbstractEventDispatcher::TimerInfo> timers = eventDispatcher->registeredTimers(this);
             if (!timers.isEmpty()) {
@@ -1354,9 +1354,9 @@ void QObjectPrivate::setThreadData_helper(QThreadData *currentData, QThreadData
             ++eventsMoved;
         }
     }
-    if (eventsMoved > 0 && targetData->eventDispatcher) {
+    if (eventsMoved > 0 && targetData->eventDispatcher.load()) {
         targetData->canWait = false;
-        targetData->eventDispatcher->wakeUp();
+        targetData->eventDispatcher.load()->wakeUp();
     }
 
     // the current emitting thread shouldn't restore currentSender after calling moveToThread()
@@ -1379,7 +1379,7 @@ void QObjectPrivate::_q_reregisterTimers(void *pointer)
 {
     Q_Q(QObject);
     QList<QAbstractEventDispatcher::TimerInfo> *timerList = reinterpret_cast<QList<QAbstractEventDispatcher::TimerInfo> *>(pointer);
-    QAbstractEventDispatcher *eventDispatcher = threadData->eventDispatcher;
+    QAbstractEventDispatcher *eventDispatcher = threadData->eventDispatcher.load();
     for (int i = 0; i < timerList->size(); ++i) {
         const QAbstractEventDispatcher::TimerInfo &ti = timerList->at(i);
         eventDispatcher->registerTimer(ti.timerId, ti.interval, ti.timerType, q);
@@ -1438,11 +1438,11 @@ int QObject::startTimer(int interval, Qt::TimerType timerType)
         return 0;
     }
 
-    if (!d->threadData->eventDispatcher) {
+    if (!d->threadData->eventDispatcher.load()) {
         qWarning("QObject::startTimer: QTimer can only be used with threads started with QThread");
         return 0;
     }
-    int timerId = d->threadData->eventDispatcher->registerTimer(interval, timerType, this);
+    int timerId = d->threadData->eventDispatcher.load()->registerTimer(interval, timerType, this);
     if (!d->extraData)
         d->extraData = new QObjectPrivate::ExtraData;
     d->extraData->runningTimers.append(timerId);
@@ -1472,8 +1472,8 @@ void QObject::killTimer(int id)
             return;
         }
 
-        if (d->threadData->eventDispatcher)
-            d->threadData->eventDispatcher->unregisterTimer(id);
+        if (d->threadData->eventDispatcher.load())
+            d->threadData->eventDispatcher.load()->unregisterTimer(id);
 
         d->extraData->runningTimers.remove(at);
         QAbstractEventDispatcherPrivate::releaseTimerId(id);
diff --git a/src/corelib/kernel/qsocketnotifier.cpp b/src/corelib/kernel/qsocketnotifier.cpp
index ad687491f1..e0c7f171c3 100644
--- a/src/corelib/kernel/qsocketnotifier.cpp
+++ b/src/corelib/kernel/qsocketnotifier.cpp
@@ -187,10 +187,10 @@ QSocketNotifier::QSocketNotifier(qintptr socket, Type type, QObject *parent)
     d->sntype = type;
     d->snenabled = true;
 
-    if (!d->threadData->eventDispatcher) {
+    if (!d->threadData->eventDispatcher.load()) {
         qWarning("QSocketNotifier: Can only be used with threads started with QThread");
     } else {
-        d->threadData->eventDispatcher->registerSocketNotifier(this);
+        d->threadData->eventDispatcher.load()->registerSocketNotifier(this);
     }
 }
 
@@ -273,12 +273,12 @@ void QSocketNotifier::setEnabled(bool enable)
         return;
     d->snenabled = enable;
 
-    if (!d->threadData->eventDispatcher) // perhaps application/thread is shutting down
+    if (!d->threadData->eventDispatcher.load()) // perhaps application/thread is shutting down
         return;
     if (d->snenabled)
-        d->threadData->eventDispatcher->registerSocketNotifier(this);
+        d->threadData->eventDispatcher.load()->registerSocketNotifier(this);
     else
-        d->threadData->eventDispatcher->unregisterSocketNotifier(this);
+        d->threadData->eventDispatcher.load()->unregisterSocketNotifier(this);
 }
 
 
diff --git a/src/corelib/kernel/qwineventnotifier.cpp b/src/corelib/kernel/qwineventnotifier.cpp
index f24843dc4a..242702b304 100644
--- a/src/corelib/kernel/qwineventnotifier.cpp
+++ b/src/corelib/kernel/qwineventnotifier.cpp
@@ -135,7 +135,7 @@ QWinEventNotifier::QWinEventNotifier(HANDLE hEvent, QObject *parent)
  : QObject(*new QWinEventNotifierPrivate(hEvent, false), parent)
 {
     Q_D(QWinEventNotifier);
-    QAbstractEventDispatcher *eventDispatcher = d->threadData->eventDispatcher;
+    QAbstractEventDispatcher *eventDispatcher = d->threadData->eventDispatcher.load();
     if (!eventDispatcher) {
         qWarning("QWinEventNotifier: Can only be used with threads started with QThread");
     } else {
@@ -208,7 +208,7 @@ void QWinEventNotifier::setEnabled(bool enable)
         return;
     d->enabled = enable;
 
-    QAbstractEventDispatcher *eventDispatcher = d->threadData->eventDispatcher;
+    QAbstractEventDispatcher *eventDispatcher = d->threadData->eventDispatcher.load();
     if (!eventDispatcher) // perhaps application is shutting down
         return;
 
diff --git a/src/corelib/thread/qthread.cpp b/src/corelib/thread/qthread.cpp
index dd4d4e74ae..d230c8a145 100644
--- a/src/corelib/thread/qthread.cpp
+++ b/src/corelib/thread/qthread.cpp
@@ -752,7 +752,7 @@ QThread::QThread(QThreadPrivate &dd, QObject *parent)
 QAbstractEventDispatcher *QThread::eventDispatcher() const
 {
     Q_D(const QThread);
-    return d->data->eventDispatcher;
+    return d->data->eventDispatcher.load();
 }
 
 /*!
@@ -767,7 +767,7 @@ QAbstractEventDispatcher *QThread::eventDispatcher() const
 void QThread::setEventDispatcher(QAbstractEventDispatcher *eventDispatcher)
 {
     Q_D(QThread);
-    if (d->data->eventDispatcher != 0) {
+    if (d->data->hasEventDispatcher()) {
         qWarning("QThread::setEventDispatcher: An event dispatcher has already been created for this thread");
     } else {
         eventDispatcher->moveToThread(this);
diff --git a/src/corelib/thread/qthread_p.h b/src/corelib/thread/qthread_p.h
index 8e9eb1035a..7e963fdcd1 100644
--- a/src/corelib/thread/qthread_p.h
+++ b/src/corelib/thread/qthread_p.h
@@ -230,6 +230,8 @@ public:
 
     void ref();
     void deref();
+    inline bool hasEventDispatcher() const
+    { return eventDispatcher.load() != 0; }
 
     bool canWaitLocked()
     {
@@ -241,7 +243,7 @@ public:
     Qt::HANDLE threadId;
     bool quitNow;
     int loopLevel;
-    QAbstractEventDispatcher *eventDispatcher;
+    QAtomicPointer<QAbstractEventDispatcher> eventDispatcher;
     QStack<QEventLoop *> eventLoops;
     QPostEventList postEventList;
     bool canWait;
diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp
index 44ad8d3ac2..b7e94366fe 100644
--- a/src/corelib/thread/qthread_unix.cpp
+++ b/src/corelib/thread/qthread_unix.cpp
@@ -258,19 +258,19 @@ typedef void*(*QtThreadCallback)(void*);
 void QThreadPrivate::createEventDispatcher(QThreadData *data)
 {
 #if defined(Q_OS_BLACKBERRY)
-    data->eventDispatcher = new QEventDispatcherBlackberry;
+    data->eventDispatcher.storeRelease(new QEventDispatcherBlackberry);
 #else
 #if !defined(QT_NO_GLIB)
     if (qEnvironmentVariableIsEmpty("QT_NO_GLIB")
         && qEnvironmentVariableIsEmpty("QT_NO_THREADED_GLIB")
         && QEventDispatcherGlib::versionSupported())
-        data->eventDispatcher = new QEventDispatcherGlib;
+        data->eventDispatcher.storeRelease(new QEventDispatcherGlib);
     else
 #endif
-    data->eventDispatcher = new QEventDispatcherUNIX;
+    data->eventDispatcher.storeRelease(new QEventDispatcherUNIX);
 #endif
 
-    data->eventDispatcher->startingUp();
+    data->eventDispatcher.load()->startingUp();
 }
 
 #ifndef QT_NO_THREAD
@@ -314,8 +314,8 @@ void *QThreadPrivate::start(void *arg)
         data->quitNow = thr->d_func()->exited;
     }
 
-    if (data->eventDispatcher) // custom event dispatcher set?
-        data->eventDispatcher->startingUp();
+    if (data->eventDispatcher.load()) // custom event dispatcher set?
+        data->eventDispatcher.load()->startingUp();
     else
         createEventDispatcher(data);
 
@@ -358,7 +358,7 @@ void QThreadPrivate::finish(void *arg)
     QThreadStorageData::finish((void **)data);
     locker.relock();
 
-    QAbstractEventDispatcher *eventDispatcher = d->data->eventDispatcher;
+    QAbstractEventDispatcher *eventDispatcher = d->data->eventDispatcher.load();
     if (eventDispatcher) {
         d->data->eventDispatcher = 0;
         locker.unlock();
diff --git a/src/corelib/thread/qthread_win.cpp b/src/corelib/thread/qthread_win.cpp
index 0cf903bb3a..ddf499f3bb 100644
--- a/src/corelib/thread/qthread_win.cpp
+++ b/src/corelib/thread/qthread_win.cpp
@@ -306,8 +306,9 @@ void qt_set_thread_name(HANDLE threadId, LPCSTR threadName)
 
 void QThreadPrivate::createEventDispatcher(QThreadData *data)
 {
-    data->eventDispatcher = new QEventDispatcherWin32;
-    data->eventDispatcher->startingUp();
+    QEventDispatcherWin32 *theEventDispatcher = new QEventDispatcherWin32;
+    data->eventDispatcher.storeRelease(theEventDispatcher);
+    theEventDispatcher->startingUp();
 }
 
 #ifndef QT_NO_THREAD
@@ -328,8 +329,8 @@ unsigned int __stdcall QT_ENSURE_STACK_ALIGNED_FOR_SSE QThreadPrivate::start(voi
         data->quitNow = thr->d_func()->exited;
     }
 
-    if (data->eventDispatcher) // custom event dispatcher set?
-        data->eventDispatcher->startingUp();
+    if (data->eventDispatcher.load()) // custom event dispatcher set?
+        data->eventDispatcher.load()->startingUp();
     else
         createEventDispatcher(data);
 
@@ -364,7 +365,7 @@ void QThreadPrivate::finish(void *arg, bool lockAnyway)
     QThreadStorageData::finish(tls_data);
     locker.relock();
 
-    QAbstractEventDispatcher *eventDispatcher = d->data->eventDispatcher;
+    QAbstractEventDispatcher *eventDispatcher = d->data->eventDispatcher.load();
     if (eventDispatcher) {
         d->data->eventDispatcher = 0;
         locker.unlock();