Fixing the race condition in event dispatcher implementation on

Symbian platform

New socket related requests are comming into QSelectThread by
interrupting the select call by writing to pipe.

One of the criteria is that m_mutex (from QSelectThread) could be
locked, meaninig that QSelectThread is in m_waitCond.wait() call.

However, the m_mutex can be locked by other contenders trying to
post new requests in burst. This would trigger writing to pipe in
false situations, making QSelectThread to hang in waitCond as
no wakeAll will come until some next request (in future) kicks in.

Task-number: QT-3358
Reviewed-by: Janne Anttila

2 files changed, 56 insertions, 10 deletions

diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp
index 6448b06..0f85a9e 100644
--- a/src/corelib/kernel/qeventdispatcher_symbian.cpp
+++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp
@@ -100,25 +100,40 @@ static inline int qt_socket_select(int nfds, fd_set *readfds, fd_set *writefds,
 class QSelectMutexGrabber
 {
 public:
-    QSelectMutexGrabber(int fd, QMutex *mutex)
-        : m_mutex(mutex)
+    QSelectMutexGrabber(int fd, QMutex *threadMutex, QMutex *selectCallMutex)
+        : m_threadMutex(threadMutex), m_selectCallMutex(selectCallMutex), bHasThreadLock(false)
     {
-        if (m_mutex->tryLock())
+        // see if selectThread is waiting m_waitCond
+        // if yes ... dont write to pipe
+        if (m_threadMutex->tryLock()) {
+            bHasThreadLock = true;
             return;
+        }
+
+        // still check that SelectThread
+        // is in select call
+        if (m_selectCallMutex->tryLock()) {
+            m_selectCallMutex->unlock();
+            return;
+        }
 
         char dummy = 0;
         qt_pipe_write(fd, &dummy, 1);
 
-        m_mutex->lock();
+        m_threadMutex->lock();
+        bHasThreadLock = true;
     }
 
     ~QSelectMutexGrabber()
     {
-        m_mutex->unlock();
+        if(bHasThreadLock)
+            m_threadMutex->unlock();
     }
 
 private:
-    QMutex *m_mutex;
+    QMutex *m_threadMutex;
+    QMutex *m_selectCallMutex;
+    bool bHasThreadLock;
 };
 
 /*
@@ -400,7 +415,12 @@ void QSelectThread::run()
 
         int ret;
         int savedSelectErrno;
-        ret = qt_socket_select(maxfd, &readfds, &writefds, &exceptionfds, 0);
+        {
+            // helps fighting the race condition between
+            // selctthread and new socket requests (cancel, restart ...)
+            QMutexLocker locker(&m_selectCallMutex);
+            ret = qt_socket_select(maxfd, &readfds, &writefds, &exceptionfds, 0);
+        }
         savedSelectErrno = errno;
 
         char buffer;
@@ -495,7 +515,9 @@ void QSelectThread::requestSocketEvents ( QSocketNotifier *notifier, TRequestSta
         start();
     }
 
-    QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex);
+    QMutexLocker locker(&m_grabberMutex);
+
+    QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex, &m_selectCallMutex);
 
     Q_ASSERT(!m_AOStatuses.contains(notifier));
 
@@ -506,7 +528,9 @@ void QSelectThread::requestSocketEvents ( QSocketNotifier *notifier, TRequestSta
 
 void QSelectThread::cancelSocketEvents ( QSocketNotifier *notifier )
 {
-    QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex);
+    QMutexLocker locker(&m_grabberMutex);
+
+    QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex, &m_selectCallMutex);
 
     m_AOStatuses.remove(notifier);
 
@@ -515,7 +539,9 @@ void QSelectThread::cancelSocketEvents ( QSocketNotifier *notifier )
 
 void QSelectThread::restart()
 {
-    QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex);
+    QMutexLocker locker(&m_grabberMutex);
+
+    QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex, &m_selectCallMutex);
 
     m_waitCond.wakeAll();
 }
diff --git a/src/corelib/kernel/qeventdispatcher_symbian_p.h b/src/corelib/kernel/qeventdispatcher_symbian_p.h
index 8a9c9a0..7021314 100644
--- a/src/corelib/kernel/qeventdispatcher_symbian_p.h
+++ b/src/corelib/kernel/qeventdispatcher_symbian_p.h
@@ -211,6 +211,26 @@ private:
     QMutex m_mutex;
     QWaitCondition m_waitCond;
     bool m_quit;
+
+    // to protect when several
+    // requests like:
+    // requestSocketEvents
+    // cancelSocketEvents
+    // kick in the same time
+    // all will fight for m_mutex
+    //
+    // TODO: fix more elegantely
+    //
+    QMutex m_grabberMutex;
+
+    // this one will tell
+    // if selectthread is
+    // really in select call
+    // and will prevent
+    // writing to pipe that
+    // causes later in locking
+    // of the thread in waitcond
+    QMutex m_selectCallMutex;
 };
 
 class Q_CORE_EXPORT CQtActiveScheduler : public CActiveScheduler