From 61daa2f419696881ccdbc1cde2dc197eaf028ff6 Mon Sep 17 00:00:00 2001 From: axis Date: Mon, 12 Jul 2010 19:45:54 +0200 Subject: Fixed a network hanging bug on Symbian. The problem happened because of a race condition in the way Qt cancelled the select call from Open C. Under high network loads, a network request could come in, causing the select call to end. Afterwards, the pipe normally used to cancel the select call would be emptied (although it was already empty). If a context switch happened after the pipe was emptied, but before the lock was released in waitCond.wait(), the main thread could try to grab the lock and write to the pipe because it was unsuccessful. This would in turn cause the next call to select to terminate immediately, and without work to do, the select thread would go straight down to waitCond.wait() and get stuck there. Fixed by moving the pipe draining loop from the select thread to the main thread, right after the lock grab. This guarantees that the select thread is empty when returning to the select call. Task: QT-3358 AutoTest: Passed RevBy: Markus Goetz --- src/corelib/kernel/qeventdispatcher_symbian.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp index c3e0808..7da1b69 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian.cpp +++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp @@ -100,16 +100,19 @@ static inline int qt_socket_select(int nfds, fd_set *readfds, fd_set *writefds, class QSelectMutexGrabber { public: - QSelectMutexGrabber(int fd, QMutex *mutex) + QSelectMutexGrabber(int writeFd, int readFd, QMutex *mutex) : m_mutex(mutex) { if (m_mutex->tryLock()) return; char dummy = 0; - qt_pipe_write(fd, &dummy, 1); + qt_pipe_write(writeFd, &dummy, 1); m_mutex->lock(); + + char buffer; + while (::read(readFd, &buffer, 1) > 0) {} } ~QSelectMutexGrabber() @@ -403,10 +406,6 @@ void QSelectThread::run() ret = qt_socket_select(maxfd, &readfds, &writefds, &exceptionfds, 0); savedSelectErrno = errno; - char buffer; - - while (::read(m_pipeEnds[0], &buffer, 1) > 0) {} - if(ret == 0) { // do nothing } else if (ret < 0) { @@ -497,7 +496,7 @@ void QSelectThread::requestSocketEvents ( QSocketNotifier *notifier, TRequestSta Q_ASSERT(QThread::currentThread() == this->thread()); - QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex); + QSelectMutexGrabber lock(m_pipeEnds[1], m_pipeEnds[0], &m_mutex); Q_ASSERT(!m_AOStatuses.contains(notifier)); @@ -510,7 +509,7 @@ void QSelectThread::cancelSocketEvents ( QSocketNotifier *notifier ) { Q_ASSERT(QThread::currentThread() == this->thread()); - QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex); + QSelectMutexGrabber lock(m_pipeEnds[1], m_pipeEnds[0], &m_mutex); m_AOStatuses.remove(notifier); @@ -521,7 +520,7 @@ void QSelectThread::restart() { Q_ASSERT(QThread::currentThread() == this->thread()); - QSelectMutexGrabber lock(m_pipeEnds[1], &m_mutex); + QSelectMutexGrabber lock(m_pipeEnds[1], m_pipeEnds[0], &m_mutex); m_waitCond.wakeAll(); } -- cgit v0.12 From fc260f7ce9c139da93e005b2b51ef37c4d23998d Mon Sep 17 00:00:00 2001 From: axis Date: Mon, 12 Jul 2010 20:06:14 +0200 Subject: Fixed a possible hanging bug in the Symbian networking. I haven't seen the bug happening in practice, but I decided to be safe rather than sorry. The rationale is that if a network request comes in, the select thread will signal the active object in the main thread, remove the socket from the set of monitored sockets, and then go to sleep in the waitCond.wait() call, waiting for reactivation by the QSelectMutexGrabber. However, in QEventDispatcherSymbian::socketFired(), if the event causes the socket to be deleted, reactivateSocketNotifier will never be called, and therefore the wait condition will never be terminated. Fixed by only entering the wait condition if a grabber has already written to the pipe, signalling that it wants the lock. AutoTest: Passed RevBy: Markus Goetz --- src/corelib/kernel/qeventdispatcher_symbian.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp index 7da1b69..1b0c31b 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian.cpp +++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp @@ -480,7 +480,8 @@ void QSelectThread::run() updateActivatedNotifiers(QSocketNotifier::Write, &writefds); } - m_waitCond.wait(&m_mutex); + if (FD_ISSET(m_pipeEnds[0], &readfds)) + m_waitCond.wait(&m_mutex); } m_mutex.unlock(); -- cgit v0.12 From 64f66cbfb924ed1810b120950273c2f58e3a2077 Mon Sep 17 00:00:00 2001 From: axis Date: Mon, 12 Jul 2010 20:24:11 +0200 Subject: Revert "Adding some error checking for setdefaultif" This reverts commit 0b56799601690a747c42dfbbefe95f18e837eb3f. Should not be necessary anymore after 61daa2f41969688. --- src/corelib/kernel/qeventdispatcher_symbian.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp index 1b0c31b..ea207f0 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian.cpp +++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp @@ -585,9 +585,7 @@ void QSelectThread::updateActivatedNotifiers(QSocketNotifier::Type type, fd_set // on some devices we do get exception // close all exiting sockets // and reset default IAP - if(::setdefaultif(0) != KErrNone) // well we can't do much about it - qWarning("setdefaultif(0) failed"); - + ::setdefaultif(0); toRemove.append(i.key()); TRequestStatus *status = i.value(); QEventDispatcherSymbian::RequestComplete(d->threadData->symbian_thread_handle, status, KErrNone); -- cgit v0.12 From b47ab877e8632f0ab2508f67d00986a0e344b7ba Mon Sep 17 00:00:00 2001 From: axis Date: Mon, 12 Jul 2010 20:25:44 +0200 Subject: Revert "Making network reconnect happen after teardown." This reverts commit 1e91d6b79cba488fa5c6f7d954de611903837f76. Should not be necessary anymore after 61daa2f41969688. --- src/corelib/kernel/qeventdispatcher_symbian.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/corelib/kernel/qeventdispatcher_symbian.cpp b/src/corelib/kernel/qeventdispatcher_symbian.cpp index ea207f0..2a52044 100644 --- a/src/corelib/kernel/qeventdispatcher_symbian.cpp +++ b/src/corelib/kernel/qeventdispatcher_symbian.cpp @@ -47,8 +47,6 @@ #include #include -#include - QT_BEGIN_NAMESPACE #ifdef SYMBIAN_GRAPHICS_WSERV_QT_EFFECTS @@ -577,15 +575,13 @@ void QSelectThread::updateActivatedNotifiers(QSocketNotifier::Type type, fd_set * check if socket is in exception set * then signal RequestComplete for it */ - qWarning("exception on %d [will do setdefaultif(0) - hack]", i.key()->socket()); + qWarning("exception on %d [will close the socket handle - hack]", i.key()->socket()); // quick fix; there is a bug // when doing read on socket // errors not preoperly mapped // after offline-ing the device // on some devices we do get exception - // close all exiting sockets - // and reset default IAP - ::setdefaultif(0); + ::close(i.key()->socket()); toRemove.append(i.key()); TRequestStatus *status = i.value(); QEventDispatcherSymbian::RequestComplete(d->threadData->symbian_thread_handle, status, KErrNone); -- cgit v0.12