From 2ef8b92ececbf9d33d7c0b44f46c7c975fb0fdaa Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Thu, 8 Jul 2010 08:42:27 +0200 Subject: QUUid::createUuid() should not generate identical sequences on UNIX An unintended side-effect of commit 90a082c9076f35dcca092ade019891e92692710e is that if qrand() is used without being seeded first, then createUuid() would always generate the same sequence. This quite likely to happen considering the Qt does not actually seed the PRNG, but does use it in many places (we do not call qsrand(), but we do often call qrand()). Fix this by changing qrand() to calculate a seed, instead of defaulting to 1. This allows us to remove the qsrand() overload with no arguments, since qrand() will now seed automatically unless manually seeded by the programmer. Task-number: QTBUG-11213 Reviewed-by: thiago --- src/corelib/global/qglobal.cpp | 58 ++++++------------------------------------ src/corelib/plugin/quuid.cpp | 12 +++------ 2 files changed, 12 insertions(+), 58 deletions(-) diff --git a/src/corelib/global/qglobal.cpp b/src/corelib/global/qglobal.cpp index 66519be..b24ba38 100644 --- a/src/corelib/global/qglobal.cpp +++ b/src/corelib/global/qglobal.cpp @@ -2586,55 +2586,6 @@ void qsrand(uint seed) #endif } -/*! \internal - \relates - \since 4.6 - - Seed the PRNG, but only if it has not already been seeded. - - The default seed is a combination of current time, a stack address and a - serial counter (since thread stack addresses are re-used). -*/ -void qsrand() -{ -#if (defined(Q_OS_UNIX) || defined(Q_OS_WIN)) && !defined(QT_NO_THREAD) - SeedStorage *seedStorage = randTLS(); - if (seedStorage) { - SeedStorageType *pseed = seedStorage->localData(); - if (pseed) { - // already seeded - return; - } - seedStorage->setLocalData(pseed = new SeedStorageType); - // start beyond 1 to avoid the sequence reset - static QBasicAtomicInt serial = Q_BASIC_ATOMIC_INITIALIZER(2); - *pseed = QDateTime::currentDateTime().toTime_t() - + quintptr(&pseed) - + serial.fetchAndAddRelaxed(1); -#if defined(Q_OS_WIN) || defined(Q_OS_SYMBIAN) - // for Windows and Symbian the srand function must still be called. - srand(*pseed); -#endif - } - -//QT_NO_THREAD implementations -#else - static unsigned int seed = 0; - - if (seed) - return; - -#if defined(Q_OS_SYMBIAN) - seed = Math::Random(); -#elif defined(Q_OS_WIN) - seed = GetTickCount(); -#else - seed = quintptr(&seed) + QDateTime::currentDateTime().toTime_t(); -#endif - srand(seed); -#endif // defined(Q_OS_UNIX) || defined(Q_OS_WIN)) && !defined(QT_NO_THREAD) -} - /*! \relates \since 4.2 @@ -2658,7 +2609,14 @@ int qrand() SeedStorageType *pseed = seedStorage->localData(); if (!pseed) { seedStorage->setLocalData(pseed = new SeedStorageType); - *pseed = 1; + + // Seed the PRNG, but only if it has not already been seeded. The + // default seed is a combination of current time, a stack address + // and a serial counter (since thread stack addresses are re-used). + static QBasicAtomicInt serial = Q_BASIC_ATOMIC_INITIALIZER(2); + *pseed = QDateTime::currentDateTime().toTime_t() + + quintptr(&pseed) + + serial.fetchAndAddRelaxed(1); } return rand_r(pseed); } else { diff --git a/src/corelib/plugin/quuid.cpp b/src/corelib/plugin/quuid.cpp index 8541c7d..6a7d35c 100644 --- a/src/corelib/plugin/quuid.cpp +++ b/src/corelib/plugin/quuid.cpp @@ -577,23 +577,19 @@ QUuid QUuid::createUuid() QT_BEGIN_INCLUDE_NAMESPACE #include "qdatetime.h" -#include "stdlib.h" // For srand/rand QT_END_INCLUDE_NAMESPACE -extern void qsrand(); // in qglobal.cpp - QUuid QUuid::createUuid() { static const int intbits = sizeof(int)*8; static int randbits = 0; if (!randbits) { + int r = 0; int max = RAND_MAX; - do { ++randbits; } while ((max=max>>1)); + do { ++r; } while ((max=max>>1)); + randbits = r; } - // reseed, but only if not already seeded - qsrand(); - QUuid result; uint *data = &(result.data1); int chunks = 16 / sizeof(uint); @@ -601,7 +597,7 @@ QUuid QUuid::createUuid() uint randNumber = 0; for (int filled = 0; filled < intbits; filled += randbits) randNumber |= qrand()< Date: Thu, 8 Jul 2010 11:47:09 +0200 Subject: Compile Re-add the stdlib.h include, since it brings in RAND_MAX Reviewed-by: TrustMe --- src/corelib/plugin/quuid.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/corelib/plugin/quuid.cpp b/src/corelib/plugin/quuid.cpp index 6a7d35c..f48cc2e 100644 --- a/src/corelib/plugin/quuid.cpp +++ b/src/corelib/plugin/quuid.cpp @@ -577,6 +577,7 @@ QUuid QUuid::createUuid() QT_BEGIN_INCLUDE_NAMESPACE #include "qdatetime.h" +#include // for RAND_MAX QT_END_INCLUDE_NAMESPACE QUuid QUuid::createUuid() -- cgit v0.12 From c48eb6d5d0a299449330dea8a6a59514942c8781 Mon Sep 17 00:00:00 2001 From: Andy Shaw Date: Thu, 8 Jul 2010 15:27:14 +0200 Subject: Initalize the nativeDialogInUse variable When the variable was not initalized it would randomly show a native font dialog or a non native one if the DontUseNativeDialogs flag was set. Task-number: QTBUG-12042 Reviewed-by: cduclos --- src/gui/dialogs/qfontdialog.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gui/dialogs/qfontdialog.cpp b/src/gui/dialogs/qfontdialog.cpp index a4bf15d..e9e3f23 100644 --- a/src/gui/dialogs/qfontdialog.cpp +++ b/src/gui/dialogs/qfontdialog.cpp @@ -332,6 +332,7 @@ void QFontDialogPrivate::init() #ifdef Q_WS_MAC delegate = 0; + nativeDialogInUse = false; #endif } -- cgit v0.12 From 8079d2951419a983df7b83cfa31f00665c3e76a3 Mon Sep 17 00:00:00 2001 From: Andy Shaw Date: Fri, 9 Jul 2010 11:52:06 +0200 Subject: Fix a couple of memory leaks due to not releasing CFTypes on Mac Reviewed-by: Prasanth Ullattil --- src/corelib/io/qfsfileengine_unix.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/corelib/io/qfsfileengine_unix.cpp b/src/corelib/io/qfsfileengine_unix.cpp index 33e00f6..76a3512 100644 --- a/src/corelib/io/qfsfileengine_unix.cpp +++ b/src/corelib/io/qfsfileengine_unix.cpp @@ -1017,7 +1017,7 @@ QString QFSFileEngine::fileName(FileName file) const #if !defined(QWS) && defined(Q_OS_MAC) QCFType url = CFURLCreateWithFileSystemPath(0, QCFString(d->filePath), kCFURLPOSIXPathStyle, true); - if (CFDictionaryRef dict = CFBundleCopyInfoDictionaryForURL(url)) { + if (QCFType dict = CFBundleCopyInfoDictionaryForURL(url)) { if (CFTypeRef name = (CFTypeRef)CFDictionaryGetValue(dict, kCFBundleNameKey)) { if (CFGetTypeID(name) == CFStringGetTypeID()) return QCFString::toQString((CFStringRef)name); @@ -1135,7 +1135,7 @@ QString QFSFileEngine::fileName(FileName file) const if (FSResolveAliasFile(&fref, true, &isFolder, &isAlias) == noErr && isAlias) { AliasHandle alias; if (FSNewAlias(0, &fref, &alias) == noErr && alias) { - CFStringRef cfstr; + QCFString cfstr; if (FSCopyAliasInfo(alias, 0, 0, &cfstr, 0, 0) == noErr) return QCFString::toQString(cfstr); } -- cgit v0.12 From 943d63e45ad09f75daa7d2f3dcc17bb28fda1766 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Fri, 9 Jul 2010 17:43:19 +0200 Subject: Fix a crash when recursing into QSharedPointer from QSharedPointer::clear() We used to delete the tracked object before the new tracking was properly set up in QSharedPointer. That means if the tracked object's destructor or deletion function recursed into the QSharedPointer, it would find itself in an inconsistent state. So instead finish the setup and only then call out to user code. That ensures the internal state is always valid. Task-number: QTBUG-11730 Reviewed-by: ossi --- src/corelib/tools/qsharedpointer_impl.h | 31 +++++++------ tests/auto/qsharedpointer/tst_qsharedpointer.cpp | 56 ++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 14 deletions(-) diff --git a/src/corelib/tools/qsharedpointer_impl.h b/src/corelib/tools/qsharedpointer_impl.h index 964b279..d581751 100644 --- a/src/corelib/tools/qsharedpointer_impl.h +++ b/src/corelib/tools/qsharedpointer_impl.h @@ -323,12 +323,17 @@ namespace QtSharedPointer { typedef ExternalRefCountData Data; inline void ref() const { d->weakref.ref(); d->strongref.ref(); } - inline bool deref() + inline void deref() + { deref(d, this->value); } + static inline void deref(Data *d, T *value) { + if (!d) return; if (!d->strongref.deref()) { - internalDestroy(); + if (!d->destroy()) + delete value; } - return d->weakref.deref(); + if (!d->weakref.deref()) + delete d; } inline void internalConstruct(T *ptr) @@ -377,7 +382,7 @@ namespace QtSharedPointer { template inline ExternalRefCount(const ExternalRefCount &other) : Basic(other.value), d(other.d) { if (d) ref(); } - inline ~ExternalRefCount() { if (d && !deref()) delete d; } + inline ~ExternalRefCount() { deref(); } template inline void internalCopy(const ExternalRefCount &other) @@ -385,12 +390,6 @@ namespace QtSharedPointer { internalSet(other.d, other.data()); } - inline void internalDestroy() - { - if (!d->destroy()) - delete this->value; - } - inline void internalSwap(ExternalRefCount &other) { qSwap(d, other.d); @@ -423,10 +422,14 @@ namespace QtSharedPointer { else o = 0; } - if (d && !deref()) - delete d; - d = o; - this->value = d && d->strongref ? actual : 0; + + qSwap(d, o); + qSwap(this->value, actual); + if (!d || d->strongref == 0) + this->value = 0; + + // dereference saved data + deref(o, actual); } Data *d; diff --git a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp index cb32c9a..4bbc9d8 100644 --- a/tests/auto/qsharedpointer/tst_qsharedpointer.cpp +++ b/tests/auto/qsharedpointer/tst_qsharedpointer.cpp @@ -92,6 +92,8 @@ private slots: void creating(); void creatingQObject(); void mixTrackingPointerCode(); + void reentrancyWhileDestructing(); + void threadStressTest_data(); void threadStressTest(); void validConstructs(); @@ -1770,6 +1772,60 @@ void tst_QSharedPointer::invalidConstructs() } } +namespace QTBUG11730 { + struct IB + { + virtual ~IB() {} + }; + + struct IA + { + virtual QSharedPointer getB() = 0; + }; + + struct B: public IB + { + IA *m_a; + B(IA *a_a) :m_a(a_a) + { } + ~B() + { + QSharedPointer b = m_a->getB(); + } + }; + + struct A: public IA + { + QSharedPointer b; + + virtual QSharedPointer getB() + { + return b; + } + + A() + { + b = QSharedPointer(new B(this)); + } + + ~A() + { + b.clear(); + } + }; +} + +void tst_QSharedPointer::reentrancyWhileDestructing() +{ + // this bug is about recursing back into QSharedPointer::clear() + // from inside it + // that is, the destructor of the object being deleted recurses + // into the same QSharedPointer object. + // First reported as QTBUG-11730 + QTBUG11730::A obj; +} + + QTEST_MAIN(tst_QSharedPointer) #include "tst_qsharedpointer.moc" -- cgit v0.12 From 29dcc069ba9a2c38b4461d5fd19d5fa9a1531595 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Mon, 12 Jul 2010 10:44:12 +0200 Subject: syncqt: fix wrong paths in include/ActiveQt/headers.pri Assigning a semicolon separated list of directories to a Qt module does not work correctly. The values of %modules must be split into single values before operating on them. This doesn't happen in all places. ActiveQt is the only module where this feature is used. Also, it is not needed anymore for this particular module. Thus, I'll just remove the semicolon separated list from ActiveQt and replace it with the module directory. Task-number: QTBUG-4586 Reviewed-by: ossi --- bin/syncqt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/syncqt b/bin/syncqt index db6dce6..4f8e343 100755 --- a/bin/syncqt +++ b/bin/syncqt @@ -42,7 +42,7 @@ my %modules = ( # path to module name map "QtScript" => "$basedir/src/script", "QtScriptTools" => "$basedir/src/scripttools", "Qt3Support" => "$basedir/src/qt3support", - "ActiveQt" => "$basedir/src/activeqt/container;$basedir/src/activeqt/control;$basedir/src/activeqt/shared", + "ActiveQt" => "$basedir/src/activeqt", "QtTest" => "$basedir/src/testlib", "QtAssistant" => "$basedir/tools/assistant/compat/lib", "QtHelp" => "$basedir/tools/assistant/lib", -- cgit v0.12 From 4ba6646150300feb79af9510c7c5d238d653a98e Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Mon, 12 Jul 2010 11:10:43 +0200 Subject: syncqt: don't try to split %module's values Using semiconlon separated directory lists in module values is broken and unused. Removing this feature. Task-number: QTBUG-4586 Reviewed-by: ossi --- bin/syncqt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/syncqt b/bin/syncqt index 4f8e343..d665e87 100755 --- a/bin/syncqt +++ b/bin/syncqt @@ -960,7 +960,7 @@ if($check_includes) { #iteration info my $lib = $_; my $dir = "$modules{$lib}"; - foreach (split(/;/, $dir)) { + { my $current_dir = "$_"; #calc subdirs my @subdirs = ($current_dir); -- cgit v0.12 From a0fffeed6fceb8244328b649a3f6feb520493bc2 Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Mon, 12 Jul 2010 14:18:29 +0200 Subject: Fix regression in tst_qrand::testqrand() qrand() has seeded with a default value of 1 for quite a long time, and is checked by the test mentioned above. The previous commit to change the default seed value must be reverted to keep compatibility. Change qrand() and qsrand() back to the way they were in 4.5. This fixes the qrand() regression. Change QUuid::createUuid() to seed exactly once per thread, which is a change from 4.5, where QUuid would see only once per application. This solves the original bug, QTBUG-3543, where multiple threads would generate the same UUID sequences. This also fixes the regression reported in QTBUG-11213, where seeding did not happen in certain cases. Reviewed-by: Prasanth Ullattil --- src/corelib/global/qglobal.cpp | 9 +-------- src/corelib/plugin/quuid.cpp | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/corelib/global/qglobal.cpp b/src/corelib/global/qglobal.cpp index b24ba38..12745e9 100644 --- a/src/corelib/global/qglobal.cpp +++ b/src/corelib/global/qglobal.cpp @@ -2609,14 +2609,7 @@ int qrand() SeedStorageType *pseed = seedStorage->localData(); if (!pseed) { seedStorage->setLocalData(pseed = new SeedStorageType); - - // Seed the PRNG, but only if it has not already been seeded. The - // default seed is a combination of current time, a stack address - // and a serial counter (since thread stack addresses are re-used). - static QBasicAtomicInt serial = Q_BASIC_ATOMIC_INITIALIZER(2); - *pseed = QDateTime::currentDateTime().toTime_t() - + quintptr(&pseed) - + serial.fetchAndAddRelaxed(1); + *pseed = 1; } return rand_r(pseed); } else { diff --git a/src/corelib/plugin/quuid.cpp b/src/corelib/plugin/quuid.cpp index f48cc2e..d0c59a4 100644 --- a/src/corelib/plugin/quuid.cpp +++ b/src/corelib/plugin/quuid.cpp @@ -577,6 +577,7 @@ QUuid QUuid::createUuid() QT_BEGIN_INCLUDE_NAMESPACE #include "qdatetime.h" +#include "qthreadstorage.h" #include // for RAND_MAX QT_END_INCLUDE_NAMESPACE @@ -591,6 +592,19 @@ QUuid QUuid::createUuid() randbits = r; } + static QThreadStorage uuidseed; + if (!uuidseed.hasLocalData()) { + // Seed the PRNG once per thread with a combination of current time, a + // stack address and a serial counter (since thread stack addresses are + // re-used). + int *pseed = new int; + static QBasicAtomicInt serial = Q_BASIC_ATOMIC_INITIALIZER(2); + qsrand(*pseed = QDateTime::currentDateTime().toTime_t() + + quintptr(&pseed) + + serial.fetchAndAddRelaxed(1)); + uuidseed.setLocalData(pseed); + } + QUuid result; uint *data = &(result.data1); int chunks = 16 / sizeof(uint); -- cgit v0.12 From a736d333aab9e2e97fdbb738b3f3f4646afe192e Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Tue, 13 Jul 2010 12:23:05 +0200 Subject: Compile when bootstrapping qmake QThreadStorage is not available when bootstrapping qmake, so fall back to a simple static bool instead. Reviewed-by: TrustMe --- src/corelib/plugin/quuid.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/corelib/plugin/quuid.cpp b/src/corelib/plugin/quuid.cpp index d0c59a4..e1d4fc0 100644 --- a/src/corelib/plugin/quuid.cpp +++ b/src/corelib/plugin/quuid.cpp @@ -592,11 +592,13 @@ QUuid QUuid::createUuid() randbits = r; } + // Seed the PRNG once per thread with a combination of current time, a + // stack address and a serial counter (since thread stack addresses are + // re-used). +#ifndef QT_BOOTSTRAPPED static QThreadStorage uuidseed; - if (!uuidseed.hasLocalData()) { - // Seed the PRNG once per thread with a combination of current time, a - // stack address and a serial counter (since thread stack addresses are - // re-used). + if (!uuidseed.hasLocalData()) + { int *pseed = new int; static QBasicAtomicInt serial = Q_BASIC_ATOMIC_INITIALIZER(2); qsrand(*pseed = QDateTime::currentDateTime().toTime_t() @@ -604,6 +606,12 @@ QUuid QUuid::createUuid() + serial.fetchAndAddRelaxed(1)); uuidseed.setLocalData(pseed); } +#else + static bool seeded = false; + if (!seeded) + qsrand(QDateTime::currentDateTime().toTime_t() + + quintptr(&seeded)); +#endif QUuid result; uint *data = &(result.data1); -- cgit v0.12