diff options
author | axis <qt-info@nokia.com> | 2010-06-24 09:58:42 (GMT) |
---|---|---|
committer | axis <qt-info@nokia.com> | 2010-06-24 09:58:42 (GMT) |
commit | 06aaa4bc68a1ee10d2a7bb8de4928c92ce84546d (patch) | |
tree | 7f20647d3f35434e44ec0a49473eb60544642f78 | |
parent | 8d05bd74dcf13045a009bfd2008795f901b74fa8 (diff) | |
download | Qt-06aaa4bc68a1ee10d2a7bb8de4928c92ce84546d.zip Qt-06aaa4bc68a1ee10d2a7bb8de4928c92ce84546d.tar.gz Qt-06aaa4bc68a1ee10d2a7bb8de4928c92ce84546d.tar.bz2 |
Revert "Fixing race condition in qeventdispatcher_symbian.cpp code path"
This reverts commit 3a9ca8ba50a63e564c15023f65cbac36d365d309.
It's the wrong fix for the problem. The socket operations in the
select thread are not supposed to be thread safe. It is thread safe
between the select thread and the caller thread, but not if there are
more than one caller thread. Ensuring complete thread safety would
probably require fixes in other areas as well.
The correct fix is for the client to ensure that calls to the socket
registering functions are either serialized or contained within one
thread.
Task: QT-3496
RevBy: Aleksandar Sasha Babic
-rw-r--r-- | src/corelib/kernel/qeventdispatcher_symbian.cpp | 101 | ||||
-rw-r--r-- | src/corelib/kernel/qeventdispatcher_symbian_p.h | 5 |
2 files changed, 40 insertions, 66 deletions
diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp index 75b7619..d00a623 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian.cpp +++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp @@ -114,13 +114,9 @@ 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); @@ -409,14 +405,11 @@ void QSelectThread::run() FD_ZERO(&exceptionfds); int maxfd = 0; - { - 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++; - } + 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); @@ -461,42 +454,40 @@ void QSelectThread::run() FD_ZERO(&readfds); FD_ZERO(&writefds); FD_ZERO(&exceptionfds); - { - QMutexLocker asoLocker(&m_AOStatusesMutex); - for (QHash<QSocketNotifier *, TRequestStatus *>::const_iterator i = m_AOStatuses.begin(); - i != m_AOStatuses.end(); ++i) { + 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 + } // end for + + // traversed all, so update + updateActivatedNotifiers(QSocketNotifier::Exception, &exceptionfds); + updateActivatedNotifiers(QSocketNotifier::Read, &readfds); + updateActivatedNotifiers(QSocketNotifier::Write, &writefds); - // 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: @@ -505,12 +496,9 @@ void QSelectThread::run() break; } } else { - { - QMutexLocker asoLocker(&m_AOStatusesMutex); - updateActivatedNotifiers(QSocketNotifier::Exception, &exceptionfds); - updateActivatedNotifiers(QSocketNotifier::Read, &readfds); - updateActivatedNotifiers(QSocketNotifier::Write, &writefds); - } + updateActivatedNotifiers(QSocketNotifier::Exception, &exceptionfds); + updateActivatedNotifiers(QSocketNotifier::Read, &readfds); + updateActivatedNotifiers(QSocketNotifier::Write, &writefds); } m_waitCond.wait(&m_mutex); @@ -529,16 +517,12 @@ void QSelectThread::requestSocketEvents ( QSocketNotifier *notifier, TRequestSta QMutexLocker locker(&m_grabberMutex); - { - // first do the thing - QMutexLocker aosLocker(&m_AOStatusesMutex); - Q_ASSERT(!m_AOStatuses.contains(notifier)); - m_AOStatuses.insert(notifier, status); - } - - // and now tell it to the others QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex, &m_selectCallMutex); + Q_ASSERT(!m_AOStatuses.contains(notifier)); + + m_AOStatuses.insert(notifier, status); + m_waitCond.wakeAll(); } @@ -546,15 +530,10 @@ void QSelectThread::cancelSocketEvents ( QSocketNotifier *notifier ) { QMutexLocker locker(&m_grabberMutex); - { - // first do the thing - QMutexLocker aosLocker(&m_AOStatusesMutex); - m_AOStatuses.remove(notifier); - } - - // and now tell it to the others QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex, &m_selectCallMutex); + m_AOStatuses.remove(notifier); + m_waitCond.wakeAll(); } diff --git a/src/corelib/kernel/qeventdispatcher_symbian_p.h b/src/corelib/kernel/qeventdispatcher_symbian_p.h index 68d5833..7021314 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian_p.h +++ b/src/corelib/kernel/qeventdispatcher_symbian_p.h @@ -231,11 +231,6 @@ 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 |