From 4519b39ba7dc25fe8b3e9cc646bfbafb0e9e0f2b Mon Sep 17 00:00:00 2001 From: Christian Prochaska Date: Mon, 2 Dec 2013 12:50:56 +0100 Subject: [PATCH] Qt5: use pthread synchronization functions The Genode-specific implementation of 'QWaitCondition' contains a race condition which can make the 'qt5_samegame' application hang on Fiasco.OC. Since most of the pthread synchronization functions needed by the UNIX-specific implementation of 'QWaitCondition' and 'QMutex' are available now, we can use these now instead of fixing and keeping the Genode-specific implementation. Fixes #993. --- libports/lib/mk/qt5_core.mk | 7 +- libports/src/lib/pthread/thread.cc | 51 +++- .../lib/qt5/patches/qt5_qtbase_genode.patch | 43 +-- .../src/corelib/thread/qmutex_genode.cpp | 150 ----------- .../corelib/thread/qwaitcondition_genode.cpp | 248 ------------------ 5 files changed, 52 insertions(+), 447 deletions(-) delete mode 100644 libports/src/lib/qt5/qtbase/src/corelib/thread/qmutex_genode.cpp delete mode 100644 libports/src/lib/qt5/qtbase/src/corelib/thread/qwaitcondition_genode.cpp diff --git a/libports/lib/mk/qt5_core.mk b/libports/lib/mk/qt5_core.mk index 6c530d3888..9f4d01c471 100644 --- a/libports/lib/mk/qt5_core.mk +++ b/libports/lib/mk/qt5_core.mk @@ -9,15 +9,12 @@ include $(REP_DIR)/lib/mk/qt5_core_generated.inc # add Genode-specific sources QT_SOURCES += qprocess_genode.cpp \ - qthread_genode.cpp \ - qwaitcondition_genode.cpp + qthread_genode.cpp # remove unsupported UNIX-specific files QT_SOURCES_FILTER_OUT = \ - qmutex_unix.cpp \ qprocess_unix.cpp \ qthread_unix.cpp \ - qwaitcondition_unix.cpp \ qfilesystemwatcher_inotify.cpp \ moc_qfilesystemwatcher_inotify_p.cpp \ @@ -34,4 +31,4 @@ INC_DIR += $(REP_DIR)/include/qt5/qtbase/QtCore/private \ $(REP_DIR)/contrib/$(QT5)/qtbase/include/QtCore/$(QT_VERSION)/QtCore \ $(REP_DIR)/contrib/$(QT5)/qtbase/include/QtCore/$(QT_VERSION)/QtCore/private -LIBS += launchpad zlib icu libc libm alarm libc_lock_pipe +LIBS += launchpad zlib icu libc libm alarm libc_lock_pipe pthread diff --git a/libports/src/lib/pthread/thread.cc b/libports/src/lib/pthread/thread.cc index 91c24a875d..02ee1f34a7 100644 --- a/libports/src/lib/pthread/thread.cc +++ b/libports/src/lib/pthread/thread.cc @@ -239,11 +239,11 @@ extern "C" { return 0; } } - + if (mutexattr.type == PTHREAD_MUTEX_ERRORCHECK) { Lock::Guard lock_guard(owner_and_counter_lock); - + if (lock_count == 0) { owner = pthread_self(); mutex_lock.lock(); @@ -408,6 +408,41 @@ extern "C" { }; + int pthread_condattr_init(pthread_condattr_t *attr) + { + if (!attr) + return EINVAL; + + PDBG("not implemented yet"); + *attr = 0; + + return 0; + } + + + int pthread_condattr_destroy(pthread_condattr_t *attr) + { + if (!attr || !*attr) + return EINVAL; + + PDBG("not implemented yet"); + + return 0; + } + + + int pthread_condattr_setclock(pthread_condattr_t *attr, + clockid_t clock_id) + { + if (!attr || !*attr) + return EINVAL; + + PDBG("not implemented yet"); + + return 0; + } + + int pthread_cond_init(pthread_cond_t *__restrict cond, const pthread_condattr_t *__restrict attr) { @@ -420,6 +455,18 @@ extern "C" { } + int pthread_cond_destroy(pthread_cond_t *cond) + { + if (!cond || !*cond) + return EINVAL; + + destroy(env()->heap(), *cond); + *cond = 0; + + return 0; + } + + static unsigned long timespec_to_ms(const struct timespec ts) { return (ts.tv_sec * 1000) + (ts.tv_nsec / (1000 * 1000)); diff --git a/libports/src/lib/qt5/patches/qt5_qtbase_genode.patch b/libports/src/lib/qt5/patches/qt5_qtbase_genode.patch index 7ffce5940f..1820e03cba 100644 --- a/libports/src/lib/qt5/patches/qt5_qtbase_genode.patch +++ b/libports/src/lib/qt5/patches/qt5_qtbase_genode.patch @@ -13,8 +13,6 @@ Genode-specific adaptations qtbase/src/corelib/kernel/qcoreapplication.cpp | 2 - .../src/corelib/kernel/qeventdispatcher_unix.cpp | 14 ++++ qtbase/src/corelib/kernel/qtranslator.cpp | 2 - - qtbase/src/corelib/thread/qmutex.cpp | 2 + - qtbase/src/corelib/thread/qmutex_p.h | 8 +++ qtbase/src/corelib/thread/qthread.cpp | 5 +- qtbase/src/corelib/thread/qthread_p.h | 55 +++++++++++++++++ qtbase/src/corelib/tools/qdatetime.cpp | 12 ++++ @@ -24,7 +22,7 @@ Genode-specific adaptations qtbase/src/network/kernel/qhostinfo_unix.cpp | 7 ++ qtbase/src/widgets/dialogs/qfiledialog.cpp | 2 - qtbase/src/widgets/styles/qstylefactory.cpp | 7 ++ - 20 files changed, 252 insertions(+), 8 deletions(-) + 18 files changed, 242 insertions(+), 8 deletions(-) diff --git a/qtbase/src/corelib/codecs/qtextcodec.cpp b/qtbase/src/corelib/codecs/qtextcodec.cpp index 1cedd3a..646be07 100644 @@ -326,45 +324,6 @@ index 9243d09..d953b20 100644 #define QT_USE_MMAP #include "private/qcore_unix_p.h" #endif -diff --git a/qtbase/src/corelib/thread/qmutex.cpp b/qtbase/src/corelib/thread/qmutex.cpp -index 1ed4a77..d1c10d1 100644 ---- a/qtbase/src/corelib/thread/qmutex.cpp -+++ b/qtbase/src/corelib/thread/qmutex.cpp -@@ -649,6 +649,8 @@ QT_END_NAMESPACE - # include "qmutex_mac.cpp" - #elif defined(Q_OS_WIN) - # include "qmutex_win.cpp" -+#elif defined(Q_OS_GENODE) -+# include "qmutex_genode.cpp" - #else - # include "qmutex_unix.cpp" - #endif -diff --git a/qtbase/src/corelib/thread/qmutex_p.h b/qtbase/src/corelib/thread/qmutex_p.h -index bec2d93..74c0df4 100644 ---- a/qtbase/src/corelib/thread/qmutex_p.h -+++ b/qtbase/src/corelib/thread/qmutex_p.h -@@ -65,6 +65,12 @@ - # include - #endif - -+#include -+ -+#ifdef Q_OS_GENODE -+#include -+#endif -+ - #if defined(Q_OS_LINUX) && !defined(QT_LINUXBASE) - // use Linux mutexes everywhere except for LSB builds - # define QT_LINUX_FUTEX -@@ -128,6 +134,8 @@ public: - //platform specific stuff - #if defined(Q_OS_MAC) - semaphore_t mach_semaphore; -+#elif defined(Q_OS_GENODE) -+ Genode::Timed_semaphore sem; - #elif defined(Q_OS_UNIX) - bool wakeup; - pthread_mutex_t mutex; diff --git a/qtbase/src/corelib/thread/qthread.cpp b/qtbase/src/corelib/thread/qthread.cpp index 4d5bee3..622056e 100644 --- a/qtbase/src/corelib/thread/qthread.cpp diff --git a/libports/src/lib/qt5/qtbase/src/corelib/thread/qmutex_genode.cpp b/libports/src/lib/qt5/qtbase/src/corelib/thread/qmutex_genode.cpp deleted file mode 100644 index 9b814a131b..0000000000 --- a/libports/src/lib/qt5/qtbase/src/corelib/thread/qmutex_genode.cpp +++ /dev/null @@ -1,150 +0,0 @@ -/**************************************************************************** -** -** Copyright (C) 2013 Digia Plc and/or its subsidiary(-ies). -** Contact: http://www.qt-project.org/legal -** -** This file is part of the QtCore module of the Qt Toolkit. -** -** $QT_BEGIN_LICENSE:LGPL$ -** Commercial License Usage -** Licensees holding valid commercial Qt licenses may use this file in -** accordance with the commercial license agreement provided with the -** Software or, alternatively, in accordance with the terms contained in -** a written agreement between you and Digia. For licensing terms and -** conditions see http://qt.digia.com/licensing. For further information -** use the contact form at http://qt.digia.com/contact-us. -** -** GNU Lesser General Public License Usage -** Alternatively, this file may be used under the terms of the GNU Lesser -** General Public License version 2.1 as published by the Free Software -** Foundation and appearing in the file LICENSE.LGPL included in the -** packaging of this file. Please review the following information to -** ensure the GNU Lesser General Public License version 2.1 requirements -** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. -** -** In addition, as a special exception, Digia gives you certain additional -** rights. These rights are described in the Digia Qt LGPL Exception -** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. -** -** GNU General Public License Usage -** Alternatively, this file may be used under the terms of the GNU -** General Public License version 3.0 as published by the Free Software -** Foundation and appearing in the file LICENSE.GPL included in the -** packaging of this file. Please review the following information to -** ensure the GNU General Public License version 3.0 requirements will be -** met: http://www.gnu.org/copyleft/gpl.html. -** -** -** $QT_END_LICENSE$ -** -****************************************************************************/ - -#include "qplatformdefs.h" -#include "qmutex.h" -#include "qstring.h" - -#ifndef QT_NO_THREAD -#include "qatomic.h" -#include "qmutex_p.h" -#include - -#ifndef Q_OS_GENODE -#if defined(Q_OS_VXWORKS) && defined(wakeup) -#undef wakeup -#endif -#endif /* Q_OS_GENODE */ - -QT_BEGIN_NAMESPACE - -#ifndef Q_OS_GENODE -static void report_error(int code, const char *where, const char *what) -{ - if (code != 0) - qWarning("%s: %s failure: %s", where, what, qPrintable(qt_error_string(code))); -} -#endif /* Q_OS_GENODE */ - -QMutexPrivate::QMutexPrivate() -#ifndef Q_OS_GENODE - : wakeup(false) -#endif /* Q_OS_GENODE */ -{ -#ifndef Q_OS_GENODE - report_error(pthread_mutex_init(&mutex, NULL), "QMutex", "mutex init"); - report_error(pthread_cond_init(&cond, NULL), "QMutex", "cv init"); -#endif /* Q_OS_GENODE */ -} - -QMutexPrivate::~QMutexPrivate() -{ -#ifndef Q_OS_GENODE - report_error(pthread_cond_destroy(&cond), "QMutex", "cv destroy"); - report_error(pthread_mutex_destroy(&mutex), "QMutex", "mutex destroy"); -#endif /* Q_OS_GENODE */ -} - -bool QMutexPrivate::wait(int timeout) -{ -#ifdef Q_OS_GENODE - bool ret; - - if (timeout == 0) { - ret = false; /* timed out */ - } else if (timeout < 0) { - sem.down(); - ret = true; /* woken up */ - } else { - try { - sem.down(timeout); - ret = true; - } catch(Genode::Timeout_exception) { - ret = false; - } - } -#else - report_error(pthread_mutex_lock(&mutex), "QMutex::lock", "mutex lock"); - int errorCode = 0; - while (!wakeup) { - if (timeout < 0) { - errorCode = pthread_cond_wait(&cond, &mutex); - } else { - struct timeval tv; - gettimeofday(&tv, 0); - timespec ti; - ti.tv_nsec = (tv.tv_usec + (timeout % 1000) * 1000) * 1000; - ti.tv_sec = tv.tv_sec + (timeout / 1000) + (ti.tv_nsec / 1000000000); - ti.tv_nsec %= 1000000000; - errorCode = pthread_cond_timedwait(&cond, &mutex, &ti); - } - if (errorCode) { - if (errorCode == ETIMEDOUT) { - if (wakeup) - errorCode = 0; - break; - } - report_error(errorCode, "QMutex::lock()", "cv wait"); - } - } - bool ret = wakeup; - wakeup = false; - report_error(pthread_mutex_unlock(&mutex), "QMutex::lock", "mutex unlock"); -#endif /* Q_OS_GENODE */ - return ret; -} - -void QMutexPrivate::wakeUp() Q_DECL_NOTHROW -{ -#ifdef Q_OS_GENODE - sem.up(); -#else - report_error(pthread_mutex_lock(&mutex), "QMutex::unlock", "mutex lock"); - wakeup = true; - report_error(pthread_cond_signal(&cond), "QMutex::unlock", "cv signal"); - report_error(pthread_mutex_unlock(&mutex), "QMutex::unlock", "mutex unlock"); -#endif /* Q_OS_GENODE */ -} - - -QT_END_NAMESPACE - -#endif // QT_NO_THREAD diff --git a/libports/src/lib/qt5/qtbase/src/corelib/thread/qwaitcondition_genode.cpp b/libports/src/lib/qt5/qtbase/src/corelib/thread/qwaitcondition_genode.cpp deleted file mode 100644 index cdbc1bc3be..0000000000 --- a/libports/src/lib/qt5/qtbase/src/corelib/thread/qwaitcondition_genode.cpp +++ /dev/null @@ -1,248 +0,0 @@ -/**************************************************************************** -** -** Copyright (C) 2013 Digia Plc and/or its subsidiary(-ies). -** Contact: http://www.qt-project.org/legal -** -** This file is part of the QtCore module of the Qt Toolkit. -** -** $QT_BEGIN_LICENSE:LGPL$ -** Commercial License Usage -** Licensees holding valid commercial Qt licenses may use this file in -** accordance with the commercial license agreement provided with the -** Software or, alternatively, in accordance with the terms contained in -** a written agreement between you and Digia. For licensing terms and -** conditions see http://qt.digia.com/licensing. For further information -** use the contact form at http://qt.digia.com/contact-us. -** -** GNU Lesser General Public License Usage -** Alternatively, this file may be used under the terms of the GNU Lesser -** General Public License version 2.1 as published by the Free Software -** Foundation and appearing in the file LICENSE.LGPL included in the -** packaging of this file. Please review the following information to -** ensure the GNU Lesser General Public License version 2.1 requirements -** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. -** -** In addition, as a special exception, Digia gives you certain additional -** rights. These rights are described in the Digia Qt LGPL Exception -** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. -** -** GNU General Public License Usage -** Alternatively, this file may be used under the terms of the GNU -** General Public License version 3.0 as published by the Free Software -** Foundation and appearing in the file LICENSE.GPL included in the -** packaging of this file. Please review the following information to -** ensure the GNU General Public License version 3.0 requirements will be -** met: http://www.gnu.org/copyleft/gpl.html. -** -** -** $QT_END_LICENSE$ -** -****************************************************************************/ - -#include "qplatformdefs.h" -#include "qwaitcondition.h" -#include "qmutex.h" -#include "qreadwritelock.h" -#include "qatomic.h" -#include "qstring.h" - -#include "qmutex_p.h" -#include "qreadwritelock_p.h" - -#include - -#ifdef Q_OS_GENODE -#include -#endif /* Q_OS_GENODE */ - -#ifndef QT_NO_THREAD - -QT_BEGIN_NAMESPACE - -#ifndef Q_OS_GENODE -static void report_error(int code, const char *where, const char *what) -{ - if (code != 0) - qWarning("%s: %s failure: %s", where, what, qPrintable(qt_error_string(code))); -} -#endif /* Q_OS_GENODE */ - - - -class QWaitConditionPrivate { -public: -#ifdef Q_OS_GENODE - Genode::Lock mutex; - Genode::Timed_semaphore sem; -#else - pthread_mutex_t mutex; - pthread_cond_t cond; - int waiters; - int wakeups; - - bool wait(unsigned long time) - { - int code; - forever { - if (time != ULONG_MAX) { - struct timeval tv; - gettimeofday(&tv, 0); - - timespec ti; - ti.tv_nsec = (tv.tv_usec + (time % 1000) * 1000) * 1000; - ti.tv_sec = tv.tv_sec + (time / 1000) + (ti.tv_nsec / 1000000000); - ti.tv_nsec %= 1000000000; - - code = pthread_cond_timedwait(&cond, &mutex, &ti); - } else { - code = pthread_cond_wait(&cond, &mutex); - } - if (code == 0 && wakeups == 0) { - // many vendors warn of spurios wakeups from - // pthread_cond_wait(), especially after signal delivery, - // even though POSIX doesn't allow for it... sigh - continue; - } - break; - } - - Q_ASSERT_X(waiters > 0, "QWaitCondition::wait", "internal error (waiters)"); - --waiters; - if (code == 0) { - Q_ASSERT_X(wakeups > 0, "QWaitCondition::wait", "internal error (wakeups)"); - --wakeups; - } - report_error(pthread_mutex_unlock(&mutex), "QWaitCondition::wait()", "mutex unlock"); - - if (code && code != ETIMEDOUT) - report_error(code, "QWaitCondition::wait()", "cv wait"); - - return (code == 0); - } -#endif /* Q_OS_GENODE */ - -}; - - -QWaitCondition::QWaitCondition() -{ - d = new QWaitConditionPrivate; -#ifndef Q_OS_GENODE - report_error(pthread_mutex_init(&d->mutex, NULL), "QWaitCondition", "mutex init"); - report_error(pthread_cond_init(&d->cond, NULL), "QWaitCondition", "cv init"); - d->waiters = d->wakeups = 0; -#endif /* Q_OS_GENODE */ -} - - -QWaitCondition::~QWaitCondition() -{ -#ifndef Q_OS_GENODE - report_error(pthread_cond_destroy(&d->cond), "QWaitCondition", "cv destroy"); - report_error(pthread_mutex_destroy(&d->mutex), "QWaitCondition", "mutex destroy"); -#endif /* Q_OS_GENODE */ - delete d; -} - -void QWaitCondition::wakeOne() -{ -#ifdef Q_OS_GENODE - Genode::Lock::Guard lock_guard(d->mutex); - - if (d->sem.cnt() < 0) { - d->sem.up(); - } -#else - report_error(pthread_mutex_lock(&d->mutex), "QWaitCondition::wakeOne()", "mutex lock"); - d->wakeups = qMin(d->wakeups + 1, d->waiters); - report_error(pthread_cond_signal(&d->cond), "QWaitCondition::wakeOne()", "cv signal"); - report_error(pthread_mutex_unlock(&d->mutex), "QWaitCondition::wakeOne()", "mutex unlock"); -#endif /* Q_OS_GENODE */ -} - -void QWaitCondition::wakeAll() -{ -#ifdef Q_OS_GENODE - Genode::Lock::Guard lock_guard(d->mutex); - - while (d->sem.cnt() < 0) { - d->sem.up(); - } -#else - report_error(pthread_mutex_lock(&d->mutex), "QWaitCondition::wakeAll()", "mutex lock"); - d->wakeups = d->waiters; - report_error(pthread_cond_broadcast(&d->cond), "QWaitCondition::wakeAll()", "cv broadcast"); - report_error(pthread_mutex_unlock(&d->mutex), "QWaitCondition::wakeAll()", "mutex unlock"); -#endif /* Q_OS_GENODE */ -} - -bool QWaitCondition::wait(QMutex *mutex, unsigned long time) -{ - if (! mutex) - return false; - if (mutex->isRecursive()) { - qWarning("QWaitCondition: cannot wait on recursive mutexes"); - return false; - } - -#ifndef Q_OS_GENODE - report_error(pthread_mutex_lock(&d->mutex), "QWaitCondition::wait()", "mutex lock"); - ++d->waiters; -#endif /* Q_OS_GENODE */ - - mutex->unlock(); - -#ifdef Q_OS_GENODE - bool returnValue; - - if (time == ULONG_MAX) { - /* Timeout: never */ - d->sem.down(); - returnValue = true; - } else { - try { - d->sem.down((Genode::Alarm::Time) time); - returnValue = true; - } catch (Genode::Timeout_exception) { - returnValue = false; - } - } -#else - bool returnValue = d->wait(time); -#endif /* Q_OS_GENODE */ - - mutex->lock(); - - return returnValue; -} - -#ifndef Q_OS_GENODE -bool QWaitCondition::wait(QReadWriteLock *readWriteLock, unsigned long time) -{ - if (!readWriteLock || readWriteLock->d->accessCount == 0) - return false; - if (readWriteLock->d->accessCount < -1) { - qWarning("QWaitCondition: cannot wait on QReadWriteLocks with recursive lockForWrite()"); - return false; - } - - report_error(pthread_mutex_lock(&d->mutex), "QWaitCondition::wait()", "mutex lock"); - ++d->waiters; - - int previousAccessCount = readWriteLock->d->accessCount; - readWriteLock->unlock(); - - bool returnValue = d->wait(time); - - if (previousAccessCount < 0) - readWriteLock->lockForWrite(); - else - readWriteLock->lockForRead(); - - return returnValue; -} -#endif /* Q_OS_GENODE */ - -QT_END_NAMESPACE - -#endif // QT_NO_THREAD