summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoraxis <qt-info@nokia.com>2010-06-24 09:58:42 (GMT)
committeraxis <qt-info@nokia.com>2010-06-24 09:58:42 (GMT)
commit06aaa4bc68a1ee10d2a7bb8de4928c92ce84546d (patch)
tree7f20647d3f35434e44ec0a49473eb60544642f78
parent8d05bd74dcf13045a009bfd2008795f901b74fa8 (diff)
downloadQt-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.cpp101
-rw-r--r--src/corelib/kernel/qeventdispatcher_symbian_p.h5
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