From 5f21450a61ba2459e6dc5b08b236b15a067bf81f Mon Sep 17 00:00:00 2001 From: Aleksandar Sasha Babic Date: Fri, 21 May 2010 13:01:00 +0200 Subject: 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 --- src/corelib/kernel/qeventdispatcher_symbian.cpp | 46 +++++++++++++++++++------ src/corelib/kernel/qeventdispatcher_symbian_p.h | 20 +++++++++++ 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 -- cgit v0.12 From 7afeed5e37c172a2a60b2d4610207243d238b0cb Mon Sep 17 00:00:00 2001 From: Janne Anttila Date: Wed, 19 May 2010 13:46:47 +0300 Subject: Fixed qsslkey test deployment for Symbian and fixed compiler warnings. Reviewed-by: Aleksandar Sasha Babic --- tests/auto/qsslkey/qsslkey.pro | 6 +++++- tests/auto/qsslkey/tst_qsslkey.cpp | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/auto/qsslkey/qsslkey.pro b/tests/auto/qsslkey/qsslkey.pro index 32138f8..a78e184 100644 --- a/tests/auto/qsslkey/qsslkey.pro +++ b/tests/auto/qsslkey/qsslkey.pro @@ -17,7 +17,11 @@ win32 { wince*|symbian: { keyFiles.sources = keys keyFiles.path = . - DEPLOYMENT += keyFiles + + passphraseFiles.sources = rsa-without-passphrase.pem rsa-with-passphrase.pem + passphraseFiles.path = . + + DEPLOYMENT += keyFiles passphraseFiles } wince*: { diff --git a/tests/auto/qsslkey/tst_qsslkey.cpp b/tests/auto/qsslkey/tst_qsslkey.cpp index 3c8ae11..0e39919 100644 --- a/tests/auto/qsslkey/tst_qsslkey.cpp +++ b/tests/auto/qsslkey/tst_qsslkey.cpp @@ -50,7 +50,7 @@ #ifdef Q_OS_SYMBIAN // In Symbian OS test data is located in applications private dir // Current path (C:\private\) contains only ascii chars -#define SRCDIR QDir::currentPath().toAscii() +#define SRCDIR "." #endif class tst_QSslKey : public QObject @@ -167,9 +167,9 @@ void tst_QSslKey::emptyConstructor() QCOMPARE(key, key2); } -Q_DECLARE_METATYPE(QSsl::KeyAlgorithm); -Q_DECLARE_METATYPE(QSsl::KeyType); -Q_DECLARE_METATYPE(QSsl::EncodingFormat); +Q_DECLARE_METATYPE(QSsl::KeyAlgorithm) +Q_DECLARE_METATYPE(QSsl::KeyType) +Q_DECLARE_METATYPE(QSsl::EncodingFormat) void tst_QSslKey::createPlainTestRows() { -- cgit v0.12 From ddcdae43cf00b49522418c2ae65022de14186ef7 Mon Sep 17 00:00:00 2001 From: Janne Anttila Date: Wed, 19 May 2010 14:04:09 +0300 Subject: Removed double EINTR loop from nativeWrite and nativeRead. qt_safe_write and and qt_safe_read already contain EINTR_LOOP. It seems that this loop has been added to code due to merge/clean-up errors in commits: 4aafbd6222e7aeafd59a4a4356ba8c53b2bfa1d1 f0a8feed9e9b4de10df76faa350201edaef98f1d Reviewed-by: Aleksandar Sasha Babic Reviewed-by: Markus Goetz --- src/network/socket/qnativesocketengine_unix.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/network/socket/qnativesocketengine_unix.cpp b/src/network/socket/qnativesocketengine_unix.cpp index 354c944..6550a50 100644 --- a/src/network/socket/qnativesocketengine_unix.cpp +++ b/src/network/socket/qnativesocketengine_unix.cpp @@ -848,11 +848,7 @@ qint64 QNativeSocketEnginePrivate::nativeWrite(const char *data, qint64 len) // Symbian does not support signals natively and Open C returns EINTR when moving to offline writtenBytes = ::write(socketDescriptor, data, len); #else - // loop while ::write() returns -1 and errno == EINTR, in case - // of an interrupting signal. - do { - writtenBytes = qt_safe_write(socketDescriptor, data, len); - } while (writtenBytes < 0 && errno == EINTR); + writtenBytes = qt_safe_write(socketDescriptor, data, len); #endif if (writtenBytes < 0) { @@ -896,9 +892,7 @@ qint64 QNativeSocketEnginePrivate::nativeRead(char *data, qint64 maxSize) #ifdef Q_OS_SYMBIAN r = ::read(socketDescriptor, data, maxSize); #else - do { - r = qt_safe_read(socketDescriptor, data, maxSize); - } while (r == -1 && errno == EINTR); + r = qt_safe_read(socketDescriptor, data, maxSize); #endif if (r < 0) { -- cgit v0.12 From c400e57c82e32d55cec0bb65465e9297927cc01e Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Fri, 21 May 2010 12:38:20 +0200 Subject: Backport multitouch bug fixes to 4.6 commit 24b811e53b30546279346ab7b16799be119ab8c4 on 4.7 includes bug fixes which are needed for 4.6 as well. 1. TouchEnd event was missing 2. pressure in touchpoints was set to 0.0 for non pressure sensitive touch screens, it should be set to 1.0 for consistency with existing Qt ports (e.g. mac). Task-number: QTBUG-10885 Reviewed-by: Bradley T. Hughes --- src/gui/kernel/qapplication_p.h | 3 ++- src/gui/kernel/qapplication_s60.cpp | 23 +++++++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/gui/kernel/qapplication_p.h b/src/gui/kernel/qapplication_p.h index ce39334..cf144c5 100644 --- a/src/gui/kernel/qapplication_p.h +++ b/src/gui/kernel/qapplication_p.h @@ -566,7 +566,8 @@ public: void _q_readRX71MultiTouchEvents(); #endif -#if defined(Q_WS_S60) +#if defined(Q_OS_SYMBIAN) + int pressureSupported; int maxTouchPressure; QList appAllTouchPoints; #endif diff --git a/src/gui/kernel/qapplication_s60.cpp b/src/gui/kernel/qapplication_s60.cpp index dbdcef9..6e6a806 100644 --- a/src/gui/kernel/qapplication_s60.cpp +++ b/src/gui/kernel/qapplication_s60.cpp @@ -407,15 +407,22 @@ void QSymbianControl::HandleLongTapEventL( const TPoint& aPenEventLocation, cons void QSymbianControl::translateAdvancedPointerEvent(const TAdvancedPointerEvent *event) { QApplicationPrivate *d = QApplicationPrivate::instance(); + qreal pressure; + if(d->pressureSupported + && event->Pressure() > 0) //workaround for misconfigured HAL + pressure = event->Pressure() / qreal(d->maxTouchPressure); + else + pressure = qreal(1.0); QRect screenGeometry = qApp->desktop()->screenGeometry(qwidget); - while (d->appAllTouchPoints.count() <= event->PointerNumber()) - d->appAllTouchPoints.append(QTouchEvent::TouchPoint(d->appAllTouchPoints.count())); + QList points = d->appAllTouchPoints; + while (points.count() <= event->PointerNumber()) + points.append(QTouchEvent::TouchPoint(points.count())); Qt::TouchPointStates allStates = 0; - for (int i = 0; i < d->appAllTouchPoints.count(); ++i) { - QTouchEvent::TouchPoint &touchPoint = d->appAllTouchPoints[i]; + for (int i = 0; i < points.count(); ++i) { + QTouchEvent::TouchPoint &touchPoint = points[i]; if (touchPoint.id() == event->PointerNumber()) { Qt::TouchPointStates state; @@ -445,7 +452,7 @@ void QSymbianControl::translateAdvancedPointerEvent(const TAdvancedPointerEvent touchPoint.setNormalizedPos(QPointF(screenPos.x() / screenGeometry.width(), screenPos.y() / screenGeometry.height())); - touchPoint.setPressure(event->Pressure() / qreal(d->maxTouchPressure)); + touchPoint.setPressure(pressure); } else if (touchPoint.state() != Qt::TouchPointReleased) { // all other active touch points should be marked as stationary touchPoint.setState(Qt::TouchPointStationary); @@ -457,11 +464,13 @@ void QSymbianControl::translateAdvancedPointerEvent(const TAdvancedPointerEvent if ((allStates & Qt::TouchPointStateMask) == Qt::TouchPointReleased) { // all touch points released d->appAllTouchPoints.clear(); + } else { + d->appAllTouchPoints = points; } QApplicationPrivate::translateRawTouchEvent(qwidget, QTouchEvent::TouchScreen, - d->appAllTouchPoints); + points); } #endif @@ -1916,6 +1925,8 @@ TUint QApplicationPrivate::resolveS60ScanCode(TInt scanCode, TUint keysym) void QApplicationPrivate::initializeMultitouch_sys() { #ifdef QT_SYMBIAN_SUPPORTS_ADVANCED_POINTER + if (HAL::Get(HALData::EPointer3DPressureSupported, pressureSupported) != KErrNone) + pressureSupported = 0; if (HAL::Get(HALData::EPointer3DMaxPressure, maxTouchPressure) != KErrNone) maxTouchPressure = KMaxTInt; #endif -- cgit v0.12