QHostInfo: fix a race condition on QHostInfoCache::enabled

In auto-test-enabled builds, QHostInfoCache can be enabled and
disabled using qt_qhostinfo_enable_cache() at any time. We cannot rule
out that users use this function, or, indeed, that the auto-test never
gets a threading stress-test. Under the assumption, then, that
QHostInfoCache::enabled can be set by any thread at any time, and is
read by any thread using QHostInfo::lookupHost(), we're presented with
a data race, thus UB.

Fix by making the accesses to QHostInfoCache::enabed atomic. Relaxed
operations are suffcient, as the bool is the only data of interest in
these situations. In particular, access to the cache itself is
protected by the cache's mutex.

We use std::atomic<bool> because QAtomicInteger<bool> doesn't exist on
all implementations, but std::atomic<bool> must. Commit a0faf9e2366 set
the precedent that it works.

Change-Id: Ia1766753bb54c5fe8d8447b51a49a96a7a853eef
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
This commit is contained in:
Marc Mutz 2019-07-04 15:24:13 +02:00
parent 1c6828b9d6
commit 0a3107dd30
2 changed files with 7 additions and 17 deletions

View File

@ -952,23 +952,10 @@ void qt_qhostinfo_cache_inject(const QString &hostname, const QHostInfo &resolut
QHostInfoCache::QHostInfoCache() : max_age(60), enabled(true), cache(128) QHostInfoCache::QHostInfoCache() : max_age(60), enabled(true), cache(128)
{ {
#ifdef QT_QHOSTINFO_CACHE_DISABLED_BY_DEFAULT #ifdef QT_QHOSTINFO_CACHE_DISABLED_BY_DEFAULT
enabled = false; enabled.store(false, std::memory_order_relaxed);
#endif #endif
} }
bool QHostInfoCache::isEnabled()
{
return enabled;
}
// this function is currently only used for the auto tests
// and not usable by public API
void QHostInfoCache::setEnabled(bool e)
{
enabled = e;
}
QHostInfo QHostInfoCache::get(const QString &name, bool *valid) QHostInfo QHostInfoCache::get(const QString &name, bool *valid)
{ {
QMutexLocker locker(&this->mutex); QMutexLocker locker(&this->mutex);

View File

@ -73,6 +73,7 @@
#include <QNetworkSession> #include <QNetworkSession>
#include <QSharedPointer> #include <QSharedPointer>
#include <atomic>
QT_BEGIN_NAMESPACE QT_BEGIN_NAMESPACE
@ -176,10 +177,12 @@ public:
void put(const QString &name, const QHostInfo &info); void put(const QString &name, const QHostInfo &info);
void clear(); void clear();
bool isEnabled(); bool isEnabled() { return enabled.load(std::memory_order_relaxed); }
void setEnabled(bool e); // this function is currently only used for the auto tests
// and not usable by public API
void setEnabled(bool e) { enabled.store(e, std::memory_order_relaxed); }
private: private:
bool enabled; std::atomic<bool> enabled;
struct QHostInfoCacheElement { struct QHostInfoCacheElement {
QHostInfo info; QHostInfo info;
QElapsedTimer age; QElapsedTimer age;