Fix construction races in QtNetwork

When two threads construct a QNetworkAccessManager at exactly the
same time on an SMP system, there are construction races for some
Q_GLOBAL_STATIC data. This is normal and expected - the losing
thread deletes its instance as part of the Q_GLOBAL_STATIC macro.

For QNetworkAccessBackendFactoryData, a guard mechanism intended
to prevent the data being reconstructed by destructors of other
global static classes was being set by the loser.
To fix this, the bool is changed to a QAtomicInt. In the normal
case, it will have value 0->1 on startup and 1->0 on shutdown.
In the race case, it will have values 0->1->2->1 on startup and
1->0 on shutdown.

Task-Number: QTBUG-20343

Change-Id: Ie3fe38944d10809d1ccdbe772df82d67faffe19c
Reviewed-on: http://codereview.qt-project.org/6181
Sanity-Review: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Peter Hartmann <peter.hartmann@nokia.com>
This commit is contained in:
Shane Kearns 2011-10-06 15:40:34 +01:00 committed by Qt by Nokia
parent 32ec981745
commit 82d897febf

View File

@ -57,20 +57,25 @@
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
static bool factoryDataShutdown = false;
class QNetworkAccessBackendFactoryData: public QList<QNetworkAccessBackendFactory *> class QNetworkAccessBackendFactoryData: public QList<QNetworkAccessBackendFactory *>
{ {
public: public:
QNetworkAccessBackendFactoryData() : mutex(QMutex::Recursive) { } QNetworkAccessBackendFactoryData() : mutex(QMutex::Recursive)
{
valid.ref();
}
~QNetworkAccessBackendFactoryData() ~QNetworkAccessBackendFactoryData()
{ {
QMutexLocker locker(&mutex); // why do we need to lock? QMutexLocker locker(&mutex); // why do we need to lock?
factoryDataShutdown = true; valid.deref();
} }
QMutex mutex; QMutex mutex;
//this is used to avoid (re)constructing factory data from destructors of other global classes
static QAtomicInt valid;
}; };
Q_GLOBAL_STATIC(QNetworkAccessBackendFactoryData, factoryData) Q_GLOBAL_STATIC(QNetworkAccessBackendFactoryData, factoryData)
QAtomicInt QNetworkAccessBackendFactoryData::valid;
QNetworkAccessBackendFactory::QNetworkAccessBackendFactory() QNetworkAccessBackendFactory::QNetworkAccessBackendFactory()
{ {
@ -80,7 +85,7 @@ QNetworkAccessBackendFactory::QNetworkAccessBackendFactory()
QNetworkAccessBackendFactory::~QNetworkAccessBackendFactory() QNetworkAccessBackendFactory::~QNetworkAccessBackendFactory()
{ {
if (!factoryDataShutdown) { if (QNetworkAccessBackendFactoryData::valid) {
QMutexLocker locker(&factoryData()->mutex); QMutexLocker locker(&factoryData()->mutex);
factoryData()->removeAll(this); factoryData()->removeAll(this);
} }
@ -89,7 +94,7 @@ QNetworkAccessBackendFactory::~QNetworkAccessBackendFactory()
QNetworkAccessBackend *QNetworkAccessManagerPrivate::findBackend(QNetworkAccessManager::Operation op, QNetworkAccessBackend *QNetworkAccessManagerPrivate::findBackend(QNetworkAccessManager::Operation op,
const QNetworkRequest &request) const QNetworkRequest &request)
{ {
if (!factoryDataShutdown) { if (QNetworkAccessBackendFactoryData::valid) {
QMutexLocker locker(&factoryData()->mutex); QMutexLocker locker(&factoryData()->mutex);
QNetworkAccessBackendFactoryData::ConstIterator it = factoryData()->constBegin(), QNetworkAccessBackendFactoryData::ConstIterator it = factoryData()->constBegin(),
end = factoryData()->constEnd(); end = factoryData()->constEnd();