Fixing race condition in qeventdispatcher_symbian.cpp code path

Fixing QT-3496. There was race condition for m_AOStatuses between
external and QSelectThread.

Task-number: QT-3496
Reviewed-by: TrustMe

2 files changed, 66 insertions, 40 deletions

diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp
index d00a623..75b7619 100644
--- a/src/corelib/kernel/qeventdispatcher_symbian.cpp
+++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp
@@ -114,9 +114,13 @@ public:
         // is in select call
         if (m_selectCallMutex->tryLock()) {
             m_selectCallMutex->unlock();
+
+            //m_threadMutex->lock();
+            //bHasThreadLock = true;
             return;
         }
 
+
         char dummy = 0;
         qt_pipe_write(fd, &dummy, 1);
 
@@ -405,11 +409,14 @@ void QSelectThread::run()
         FD_ZERO(&exceptionfds);
 
         int maxfd = 0;
-        maxfd = qMax(maxfd, updateSocketSet(QSocketNotifier::Read, &readfds));
-        maxfd = qMax(maxfd, updateSocketSet(QSocketNotifier::Write, &writefds));
-        maxfd = qMax(maxfd, updateSocketSet(QSocketNotifier::Exception, &exceptionfds));
-        maxfd = qMax(maxfd, m_pipeEnds[0]);
-        maxfd++;
+        {
+            QMutexLocker asoLocker(&m_AOStatusesMutex);
+            maxfd = qMax(maxfd, updateSocketSet(QSocketNotifier::Read, &readfds));
+            maxfd = qMax(maxfd, updateSocketSet(QSocketNotifier::Write, &writefds));
+            maxfd = qMax(maxfd, updateSocketSet(QSocketNotifier::Exception, &exceptionfds));
+            maxfd = qMax(maxfd, m_pipeEnds[0]);
+            maxfd++;
+        }
 
         FD_SET(m_pipeEnds[0], &readfds);
 
@@ -454,40 +461,42 @@ void QSelectThread::run()
                 FD_ZERO(&readfds);
                 FD_ZERO(&writefds);
                 FD_ZERO(&exceptionfds);
-                for (QHash<QSocketNotifier *, TRequestStatus *>::const_iterator i = m_AOStatuses.begin();
-                        i != m_AOStatuses.end(); ++i) {
+                {
+                    QMutexLocker asoLocker(&m_AOStatusesMutex);
+                    for (QHash<QSocketNotifier *, TRequestStatus *>::const_iterator i = m_AOStatuses.begin();
+                            i != m_AOStatuses.end(); ++i) {
 
-                    fd_set onefds;
-                    FD_ZERO(&onefds);
-                    FD_SET(i.key()->socket(), &onefds);
+                       fd_set onefds;
+                       FD_ZERO(&onefds);
+                       FD_SET(i.key()->socket(), &onefds);
 
-                    fd_set excfds;
-                    FD_ZERO(&excfds);
-                    FD_SET(i.key()->socket(), &excfds);
+                       fd_set excfds;
+                       FD_ZERO(&excfds);
+                       FD_SET(i.key()->socket(), &excfds);
 
-                    maxfd = i.key()->socket() + 1;
+                        maxfd = i.key()->socket() + 1;
 
-                    struct timeval timeout;
-                    timeout.tv_sec = 0;
-                    timeout.tv_usec = 0;
+                        struct timeval timeout;
+                        timeout.tv_sec = 0;
+                        timeout.tv_usec = 0;
 
-                    ret = 0;
+                        ret = 0;
 
-                    if(i.key()->type() == QSocketNotifier::Read) {
-                        ret = ::select(maxfd, &onefds, 0, &excfds, &timeout);
-                        if(ret != 0) FD_SET(i.key()->socket(), &readfds);
-                    } else if(i.key()->type() == QSocketNotifier::Write) {
-                        ret = ::select(maxfd, 0, &onefds, &excfds, &timeout);
-                        if(ret != 0) FD_SET(i.key()->socket(), &writefds);
-                    }
+                        if(i.key()->type() == QSocketNotifier::Read) {
+                            ret = ::select(maxfd, &onefds, 0, &excfds, &timeout);
+                            if(ret != 0) FD_SET(i.key()->socket(), &readfds);
+                        } else if(i.key()->type() == QSocketNotifier::Write) {
+                            ret = ::select(maxfd, 0, &onefds, &excfds, &timeout);
+                            if(ret != 0) FD_SET(i.key()->socket(), &writefds);
+                        }
 
-                } // end for
-
-                // traversed all, so update
-                updateActivatedNotifiers(QSocketNotifier::Exception, &exceptionfds);
-                updateActivatedNotifiers(QSocketNotifier::Read, &readfds);
-                updateActivatedNotifiers(QSocketNotifier::Write, &writefds);
+                    } // end for
 
+                    // traversed all, so update
+                    updateActivatedNotifiers(QSocketNotifier::Exception, &exceptionfds);
+                    updateActivatedNotifiers(QSocketNotifier::Read, &readfds);
+                    updateActivatedNotifiers(QSocketNotifier::Write, &writefds);
+                }
                 break;
             case EINTR: // Should never occur on Symbian, but this is future proof!
             default:
@@ -496,9 +505,12 @@ void QSelectThread::run()
                 break;
             }
         } else {
-            updateActivatedNotifiers(QSocketNotifier::Exception, &exceptionfds);
-            updateActivatedNotifiers(QSocketNotifier::Read, &readfds);
-            updateActivatedNotifiers(QSocketNotifier::Write, &writefds);
+            {
+                QMutexLocker asoLocker(&m_AOStatusesMutex);
+                updateActivatedNotifiers(QSocketNotifier::Exception, &exceptionfds);
+                updateActivatedNotifiers(QSocketNotifier::Read, &readfds);
+                updateActivatedNotifiers(QSocketNotifier::Write, &writefds);
+            }
         }
 
         m_waitCond.wait(&m_mutex);
@@ -517,11 +529,15 @@ void QSelectThread::requestSocketEvents ( QSocketNotifier *notifier, TRequestSta
 
     QMutexLocker locker(&m_grabberMutex);
 
-    QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex, &m_selectCallMutex);
-
-    Q_ASSERT(!m_AOStatuses.contains(notifier));
+    {
+        // first do the thing
+        QMutexLocker aosLocker(&m_AOStatusesMutex); 
+        Q_ASSERT(!m_AOStatuses.contains(notifier));
+        m_AOStatuses.insert(notifier, status);
+    }
 
-    m_AOStatuses.insert(notifier, status);
+    // and now tell it to the others
+    QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex, &m_selectCallMutex);
 
     m_waitCond.wakeAll();
 }
@@ -530,9 +546,14 @@ void QSelectThread::cancelSocketEvents ( QSocketNotifier *notifier )
 {
     QMutexLocker locker(&m_grabberMutex);
 
-    QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex, &m_selectCallMutex);
+    {
+        // first do the thing
+        QMutexLocker aosLocker(&m_AOStatusesMutex);
+        m_AOStatuses.remove(notifier);
+    }
 
-    m_AOStatuses.remove(notifier);
+    // and now tell it to the others
+    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 7021314..68d5833 100644
--- a/src/corelib/kernel/qeventdispatcher_symbian_p.h
+++ b/src/corelib/kernel/qeventdispatcher_symbian_p.h
@@ -231,6 +231,11 @@ private:
     // causes later in locking
     // of the thread in waitcond
     QMutex m_selectCallMutex;
+
+    // Argh ... not again
+    // one more unprotected
+    // resource is m_AOStatuses
+    QMutex m_AOStatusesMutex;
 };
 
 class Q_CORE_EXPORT CQtActiveScheduler : public CActiveScheduler