From 90a082c9076f35dcca092ade019891e92692710e Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Fri, 9 Oct 2009 12:13:47 +0200 Subject: QUuid::createUuid() not unique when using threads on Unix QUuid::createUuid() only seeds the PRNG on the first entry, but since it's using qsrand() and qrand(), all other threads will use the default seed, and thus generate the exact same UUIDs. Fix this by adding an internal function (qsrand() overload with no args) which seeds the PRNG if it hasn't been done already, and use a seed that is based on current time, a stack address and a global serial counter (so that the chances of 2 threads using the same seed are as low as possible). Task-number: QTBUG-3543 Reviewed-by: Marius Storm-Olsen --- src/corelib/global/qglobal.cpp | 28 ++++++++++++++++++++++++++++ src/corelib/plugin/quuid.cpp | 13 +++++++++---- tests/auto/quuid/tst_quuid.cpp | 26 ++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/corelib/global/qglobal.cpp b/src/corelib/global/qglobal.cpp index 742f4ec..9fc3c8d 100644 --- a/src/corelib/global/qglobal.cpp +++ b/src/corelib/global/qglobal.cpp @@ -46,6 +46,7 @@ #include "qthreadstorage.h" #include "qdir.h" #include "qstringlist.h" +#include "qdatetime.h" #ifndef QT_NO_QOBJECT #include @@ -2523,6 +2524,33 @@ 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(QT_NO_THREAD) && !defined(Q_OS_SYMBIAN) + SeedStorageType *pseed = randTLS()->localData(); + if (pseed) { + // already seeded + return; + } + randTLS()->setLocalData(pseed = new SeedStorageType); + static QBasicAtomicInt serial = Q_BASIC_ATOMIC_INITIALIZER(0); + *pseed = QDateTime::currentDateTime().toTime_t() + + quintptr(&pseed) + + serial.fetchAndAddRelaxed(1); +#else + // On Windows, we assume that rand() already does the right thing +#endif +} + /*! \relates \since 4.2 diff --git a/src/corelib/plugin/quuid.cpp b/src/corelib/plugin/quuid.cpp index 7224ad3..3c79653 100644 --- a/src/corelib/plugin/quuid.cpp +++ b/src/corelib/plugin/quuid.cpp @@ -548,9 +548,11 @@ bool QUuid::operator>(const QUuid &other) const On any platform other than Windows, this function returns a new UUID with variant QUuid::DCE and version QUuid::Random. The random numbers used to construct the UUID are obtained from the local - pseudo-random generator, which is usually not a cryptographic + pseudo-random generator, qrand(), which is usually not a cryptographic quality random number generator. Therefore, a UUID generated by - this function can't be guaranteed to be unique. + this function can't be guaranteed to be unique. If the pseudo-random + number generator for the calling thread has not yet been seeded, this + function will seed the pseudo-random number generator by calling qsrand(). On a Windows platform, a GUID is generated, which almost certainly \e{will} be unique, on this or any other system, networked or not. @@ -578,6 +580,8 @@ QT_BEGIN_INCLUDE_NAMESPACE #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; @@ -585,10 +589,11 @@ QUuid QUuid::createUuid() if (!randbits) { int max = RAND_MAX; do { ++randbits; } while ((max=max>>1)); - qsrand((uint)QDateTime::currentDateTime().toTime_t()); - qrand(); // Skip first } + // reseed, but only if not already seeded + qsrand(); + QUuid result; uint *data = &(result.data1); int chunks = 16 / sizeof(uint); diff --git a/tests/auto/quuid/tst_quuid.cpp b/tests/auto/quuid/tst_quuid.cpp index e262be7..d78fda5 100644 --- a/tests/auto/quuid/tst_quuid.cpp +++ b/tests/auto/quuid/tst_quuid.cpp @@ -72,6 +72,8 @@ private slots: void variants(); void versions(); + void threadUniqueness(); + public: // Variables QUuid uuidA; @@ -169,6 +171,30 @@ void tst_QUuid::versions() QVERIFY( NCS.version() == QUuid::VerUnknown ); } +class UuidThread : public QThread +{ +public: + QUuid uuid; + + void run() + { + uuid = QUuid::createUuid(); + } +}; + +void tst_QUuid::threadUniqueness() +{ + QVector threads(qMax(2, QThread::idealThreadCount())); + for (int i = 0; i < threads.count(); ++i) + threads[i] = new UuidThread; + for (int i = 0; i < threads.count(); ++i) + threads[i]->start(); + for (int i = 0; i < threads.count(); ++i) + QVERIFY(threads[i]->wait(1000)); + for (int i = 1; i < threads.count(); ++i) + QVERIFY(threads[0]->uuid != threads[i]->uuid); + qDeleteAll(threads); +} QTEST_MAIN(tst_QUuid) #include "tst_quuid.moc" -- cgit v0.12