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

1 files changed, 36 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();
 }