From c3fe37fe18961d3ddb050fc788281eb79bc61d7a Mon Sep 17 00:00:00 2001 From: David Faure Date: Sat, 14 Jul 2012 00:39:18 +0200 Subject: QUrl: fix thread safety. Developers expect const methods on the same QUrl instance to be usable from multiple threads (including copying and modifying the copy). The delayed parsing and internal cache of encoded strings break this, however (and the implicit sharing, for the case of copying). Protection with a mutex fixes this. Qt-5.0 doesn't have this issue, since QUrl doesn't do delayed parsing anymore. Change-Id: Ie2674e46eb7416b0e753323b673c10f9d3457f6d Reviewed-by: Marc Mutz Reviewed-by: Thiago Macieira --- src/corelib/io/qurl.cpp | 228 +++++++++++++++++++++++++++++++++---------- src/corelib/io/qurl.h | 2 + tests/auto/qurl/tst_qurl.cpp | 83 ++++++++++++++++ 3 files changed, 264 insertions(+), 49 deletions(-) diff --git a/src/corelib/io/qurl.cpp b/src/corelib/io/qurl.cpp index dcca931..e9dfffa 100644 --- a/src/corelib/io/qurl.cpp +++ b/src/corelib/io/qurl.cpp @@ -212,6 +212,8 @@ #if defined(Q_OS_WINCE_WM) #pragma optimize("g", off) #endif +#include "qmutex.h" +#include "private/qorderedmutexlocker_p.h" QT_BEGIN_NAMESPACE @@ -322,6 +324,8 @@ public: QString userInfo(QUrl::FormattingOptions options = QUrl::None) const; void setEncodedAuthority(const QByteArray &authority); void setEncodedUserInfo(const QUrlParseData *parseData); + void setEncodedUrl(const QByteArray &encodedUrl, QUrl::ParsingMode parsingMode); + QString fragmentImpl() const; QByteArray mergePaths(const QByteArray &relativePath) const; @@ -373,6 +377,13 @@ public: }; int stateFlags; + // This mutex protects stateFlags, errorInfo, host, + // hasQuery, hasFragment, isValid, isHostValid, + // encodedNormalized and all other encoded* variables above. + // This also protects all other variables when set by parse, + // but not when set by setters. + mutable QMutex mutex; + mutable QByteArray encodedNormalized; const QByteArray & normalized() const; @@ -3426,6 +3437,7 @@ QUrlPrivate::QUrlPrivate() hasQuery = false; } +// Called by normalized() and detach(). Must hold copy.mutex. QUrlPrivate::QUrlPrivate(const QUrlPrivate ©) : scheme(copy.scheme), userName(copy.userName), @@ -3453,6 +3465,7 @@ QUrlPrivate::QUrlPrivate(const QUrlPrivate ©) QString QUrlPrivate::canonicalHost() const { + // Caller must lock mutex first if (QURL_HASFLAG(stateFlags, HostCanonicalized) || host.isEmpty()) return host; @@ -3541,6 +3554,8 @@ void QUrlPrivate::ensureEncodedParts() const QString QUrlPrivate::authority(QUrl::FormattingOptions options) const { + // Caller must lock mutex first + if ((options & QUrl::RemoveAuthority) == QUrl::RemoveAuthority) return QString(); @@ -3668,6 +3683,8 @@ QString QUrlPrivate::userInfo(QUrl::FormattingOptions options) const */ QByteArray QUrlPrivate::mergePaths(const QByteArray &relativePath) const { + // Caller must lock mutex first + if (encodedPath.isNull()) ensureEncodedParts(); @@ -3784,7 +3801,8 @@ static void removeDotsFromPath(QByteArray *path) void QUrlPrivate::validate() const { - QUrlPrivate *that = (QUrlPrivate *)this; + // Caller must lock mutex first + QUrlPrivate *that = const_cast(this); that->encodedOriginal = that->toEncoded(); // may detach parse(ParseOnly); @@ -3817,7 +3835,8 @@ void QUrlPrivate::validate() const void QUrlPrivate::parse(ParseOptions parseOptions) const { - QUrlPrivate *that = (QUrlPrivate *)this; + // Caller must lock mutex first + QUrlPrivate *that = const_cast(this); that->errorInfo.setParams(0, 0, 0, 0); if (encodedOriginal.isEmpty()) { that->isValid = false; @@ -3960,6 +3979,7 @@ void QUrlPrivate::clear() QByteArray QUrlPrivate::toEncoded(QUrl::FormattingOptions options) const { + // Caller must lock mutex first if (!QURL_HASFLAG(stateFlags, Parsed)) parse(); else ensureEncodedParts(); @@ -4045,6 +4065,7 @@ QByteArray QUrlPrivate::toEncoded(QUrl::FormattingOptions options) const const QByteArray &QUrlPrivate::normalized() const { + // Caller must lock mutex first if (QURL_HASFLAG(stateFlags, QUrlPrivate::Normalized)) return encodedNormalized; @@ -4264,6 +4285,7 @@ bool QUrl::isValid() const { if (!d) return false; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Validated)) d->validate(); @@ -4277,6 +4299,7 @@ bool QUrl::isEmpty() const { if (!d) return true; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) return d->encodedOriginal.isEmpty(); else @@ -4327,11 +4350,15 @@ void QUrl::setUrl(const QString &url) */ void QUrl::setUrl(const QString &url, ParsingMode parsingMode) { - detach(); + if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); + detach(lock); + d->clear(); + // escape all reserved characters and delimiters // reserved = gen-delims / sub-delims if (parsingMode != TolerantMode) { - setEncodedUrl(toPercentEncodingHelper(url, ABNF_reserved), parsingMode); + d->setEncodedUrl(toPercentEncodingHelper(url, ABNF_reserved), parsingMode); return; } @@ -4364,7 +4391,7 @@ void QUrl::setUrl(const QString &url, ParsingMode parsingMode) } else { encodedUrl = toPercentEncodingHelper(tmp, ABNF_reserved); } - setEncodedUrl(encodedUrl, StrictMode); + d->setEncodedUrl(encodedUrl, StrictMode); } /*! @@ -4399,10 +4426,18 @@ static inline char toHex(quint8 c) */ void QUrl::setEncodedUrl(const QByteArray &encodedUrl, ParsingMode parsingMode) { - QByteArray tmp = encodedUrl; - detach(); + if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); + detach(lock); d->clear(); - if ((d->parsingMode = parsingMode) == TolerantMode) { + d->setEncodedUrl(encodedUrl, parsingMode); +} + +void QUrlPrivate::setEncodedUrl(const QByteArray &encodedUrl, QUrl::ParsingMode parsingMode) +{ + // Caller must lock mutex first + QByteArray tmp = encodedUrl; + if ((this->parsingMode = parsingMode) == QUrl::TolerantMode) { // Replace stray % with %25 QByteArray copy = tmp; for (int i = 0, j = 0; i < copy.size(); ++i, ++j) { @@ -4453,7 +4488,7 @@ void QUrl::setEncodedUrl(const QByteArray &encodedUrl, ParsingMode parsingMode) } } - d->encodedOriginal = tmp; + encodedOriginal = tmp; } /*! @@ -4476,8 +4511,9 @@ void QUrl::setEncodedUrl(const QByteArray &encodedUrl, ParsingMode parsingMode) void QUrl::setScheme(const QString &scheme) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->scheme = scheme; @@ -4492,6 +4528,7 @@ void QUrl::setScheme(const QString &scheme) QString QUrl::scheme() const { if (!d) return QString(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); return d->scheme; @@ -4517,8 +4554,9 @@ void QUrl::setAuthority(const QString &authority) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized | QUrlPrivate::HostCanonicalized); d->setAuthority(authority); } @@ -4533,6 +4571,7 @@ QString QUrl::authority() const { if (!d) return QString(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); return d->authority(); @@ -4555,8 +4594,9 @@ void QUrl::setUserInfo(const QString &userInfo) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->setUserInfo(userInfo.trimmed()); @@ -4570,6 +4610,7 @@ QString QUrl::userInfo() const { if (!d) return QString(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); return d->userInfo(); @@ -4586,8 +4627,9 @@ void QUrl::setUserName(const QString &userName) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->userName = userName; @@ -4604,6 +4646,7 @@ QString QUrl::userName() const { if (!d) return QString(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); d->userInfo(); // causes the unencoded form to be set @@ -4626,8 +4669,9 @@ QString QUrl::userName() const void QUrl::setEncodedUserName(const QByteArray &userName) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->encodedUserName = userName; @@ -4647,6 +4691,7 @@ void QUrl::setEncodedUserName(const QByteArray &userName) QByteArray QUrl::encodedUserName() const { if (!d) return QByteArray(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); d->ensureEncodedParts(); @@ -4663,8 +4708,9 @@ QByteArray QUrl::encodedUserName() const void QUrl::setPassword(const QString &password) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->password = password; @@ -4680,6 +4726,7 @@ void QUrl::setPassword(const QString &password) QString QUrl::password() const { if (!d) return QString(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); d->userInfo(); // causes the unencoded form to be set @@ -4702,8 +4749,9 @@ QString QUrl::password() const void QUrl::setEncodedPassword(const QByteArray &password) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->encodedPassword = password; @@ -4723,6 +4771,7 @@ void QUrl::setEncodedPassword(const QByteArray &password) QByteArray QUrl::encodedPassword() const { if (!d) return QByteArray(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); d->ensureEncodedParts(); @@ -4738,8 +4787,9 @@ QByteArray QUrl::encodedPassword() const void QUrl::setHost(const QString &host) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); d->isHostValid = true; QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized | QUrlPrivate::HostCanonicalized); @@ -4753,6 +4803,7 @@ void QUrl::setHost(const QString &host) QString QUrl::host() const { if (!d) return QString(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); QString result = d->canonicalHost(); @@ -4806,8 +4857,9 @@ QByteArray QUrl::encodedHost() const void QUrl::setPort(int port) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); if (port < -1 || port > 65535) { @@ -4824,6 +4876,7 @@ void QUrl::setPort(int port) int QUrl::port() const { if (!d) return -1; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Validated)) d->validate(); return d->port; @@ -4843,6 +4896,7 @@ int QUrl::port() const int QUrl::port(int defaultPort) const { if (!d) return defaultPort; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); return d->port == -1 ? defaultPort : d->port; } @@ -4863,8 +4917,9 @@ int QUrl::port(int defaultPort) const void QUrl::setPath(const QString &path) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->path = path; @@ -4879,6 +4934,7 @@ void QUrl::setPath(const QString &path) QString QUrl::path() const { if (!d) return QString(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); if (d->path.isNull()) { @@ -4911,8 +4967,9 @@ QString QUrl::path() const void QUrl::setEncodedPath(const QByteArray &path) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->encodedPath = path; @@ -4932,6 +4989,7 @@ void QUrl::setEncodedPath(const QByteArray &path) QByteArray QUrl::encodedPath() const { if (!d) return QByteArray(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); d->ensureEncodedParts(); @@ -4948,6 +5006,7 @@ QByteArray QUrl::encodedPath() const bool QUrl::hasQuery() const { if (!d) return false; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); return d->hasQuery; @@ -4978,7 +5037,8 @@ bool QUrl::hasQuery() const void QUrl::setQueryDelimiters(char valueDelimiter, char pairDelimiter) { if (!d) d = new QUrlPrivate; - detach(); + QMutexLocker lock(&d->mutex); + detach(lock); d->valueDelimiter = valueDelimiter; d->pairDelimiter = pairDelimiter; @@ -5024,8 +5084,9 @@ char QUrl::queryValueDelimiter() const void QUrl::setEncodedQuery(const QByteArray &query) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->query = query; @@ -5048,8 +5109,9 @@ void QUrl::setEncodedQuery(const QByteArray &query) void QUrl::setQueryItems(const QList > &query) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); char alsoEncode[3]; alsoEncode[0] = d->valueDelimiter; @@ -5088,8 +5150,9 @@ void QUrl::setQueryItems(const QList > &query) void QUrl::setEncodedQueryItems(const QList > &query) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QByteArray queryTmp; for (int i = 0; i < query.size(); i++) { @@ -5122,8 +5185,9 @@ void QUrl::setEncodedQueryItems(const QList > &que void QUrl::addQueryItem(const QString &key, const QString &value) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); char alsoEncode[3]; alsoEncode[0] = d->valueDelimiter; @@ -5157,8 +5221,9 @@ void QUrl::addQueryItem(const QString &key, const QString &value) void QUrl::addEncodedQueryItem(const QByteArray &key, const QByteArray &value) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); if (!d->query.isEmpty()) d->query += d->pairDelimiter; @@ -5182,6 +5247,7 @@ void QUrl::addEncodedQueryItem(const QByteArray &key, const QByteArray &value) QList > QUrl::queryItems() const { if (!d) return QList >(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); QList > itemMap; @@ -5215,6 +5281,7 @@ QList > QUrl::queryItems() const QList > QUrl::encodedQueryItems() const { if (!d) return QList >(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); QList > itemMap; @@ -5263,6 +5330,7 @@ bool QUrl::hasQueryItem(const QString &key) const bool QUrl::hasEncodedQueryItem(const QByteArray &key) const { if (!d) return false; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); int pos = 0; @@ -5310,6 +5378,7 @@ QString QUrl::queryItemValue(const QString &key) const QByteArray QUrl::encodedQueryItemValue(const QByteArray &key) const { if (!d) return QByteArray(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); int pos = 0; @@ -5338,6 +5407,7 @@ QByteArray QUrl::encodedQueryItemValue(const QByteArray &key) const QStringList QUrl::allQueryItemValues(const QString &key) const { if (!d) return QStringList(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); QByteArray encodedKey = toPercentEncoding(key, queryExcludeChars); @@ -5376,6 +5446,7 @@ QStringList QUrl::allQueryItemValues(const QString &key) const QList QUrl::allEncodedQueryItemValues(const QByteArray &key) const { if (!d) return QList(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); QList values; @@ -5423,8 +5494,9 @@ void QUrl::removeQueryItem(const QString &key) void QUrl::removeEncodedQueryItem(const QByteArray &key) { if (!d) return; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); int pos = 0; const char *query = d->query.constData(); @@ -5469,8 +5541,9 @@ void QUrl::removeAllQueryItems(const QString &key) void QUrl::removeAllEncodedQueryItems(const QByteArray &key) { if (!d) return; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); int pos = 0; const char *query = d->query.constData(); @@ -5494,6 +5567,7 @@ void QUrl::removeAllEncodedQueryItems(const QByteArray &key) QByteArray QUrl::encodedQuery() const { if (!d) return QByteArray(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); return d->query; @@ -5519,8 +5593,9 @@ QByteArray QUrl::encodedQuery() const void QUrl::setFragment(const QString &fragment) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->fragment = fragment; @@ -5536,13 +5611,19 @@ void QUrl::setFragment(const QString &fragment) QString QUrl::fragment() const { if (!d) return QString(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - if (d->fragment.isNull() && !d->encodedFragment.isNull()) { - QUrlPrivate *that = const_cast(d); - that->fragment = fromPercentEncodingHelper(d->encodedFragment); + return d->fragmentImpl(); +} + +QString QUrlPrivate::fragmentImpl() const +{ + if (fragment.isNull() && !encodedFragment.isNull()) { + QUrlPrivate *that = const_cast(this); + that->fragment = fromPercentEncodingHelper(encodedFragment); } - return d->fragment; + return fragment; } /*! @@ -5567,8 +5648,9 @@ QString QUrl::fragment() const void QUrl::setEncodedFragment(const QByteArray &fragment) { if (!d) d = new QUrlPrivate; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); - detach(); + detach(lock); QURL_UNSETFLAG(d->stateFlags, QUrlPrivate::Validated | QUrlPrivate::Normalized); d->encodedFragment = fragment; @@ -5589,6 +5671,7 @@ void QUrl::setEncodedFragment(const QByteArray &fragment) QByteArray QUrl::encodedFragment() const { if (!d) return QByteArray(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); d->ensureEncodedParts(); @@ -5605,6 +5688,7 @@ QByteArray QUrl::encodedFragment() const bool QUrl::hasFragment() const { if (!d) return false; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); return d->hasFragment; @@ -5648,8 +5732,10 @@ QUrl QUrl::resolved(const QUrl &relative) const { if (!d) return relative; if (!relative.d) return *this; - if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); + QOrderedMutexLocker locker(&d->mutex, &relative.d->mutex); + + if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); if (!QURL_HASFLAG(relative.d->stateFlags, QUrlPrivate::Parsed)) relative.d->parse(); @@ -5660,20 +5746,24 @@ QUrl QUrl::resolved(const QUrl &relative) const // be non strict and allow scheme in relative url if (!relative.d->scheme.isEmpty() && relative.d->scheme != d->scheme) { t = relative; + // t.detach(locker) would unlock, so bypass it + qAtomicDetach(t.d); } else { - if (!relative.authority().isEmpty()) { + if (!relative.d->authority().isEmpty()) { t = relative; + qAtomicDetach(t.d); } else { t.d = new QUrlPrivate; if (relative.d->encodedPath.isEmpty()) { t.d->encodedPath = d->encodedPath; - t.setEncodedQuery(relative.d->hasQuery ? relative.d->query : d->query); + t.d->query = relative.d->hasQuery ? relative.d->query : d->query; } else { t.d->encodedPath = relative.d->encodedPath.at(0) == '/' ? relative.d->encodedPath : d->mergePaths(relative.d->encodedPath); - t.setEncodedQuery(relative.d->query); + t.d->query = relative.d->query; } + t.d->hasQuery = !t.d->query.isNull(); t.d->encodedUserName = d->encodedUserName; t.d->encodedPassword = d->encodedPassword; t.d->host = d->host; @@ -5681,14 +5771,14 @@ QUrl QUrl::resolved(const QUrl &relative) const } t.setScheme(d->scheme); } - t.setFragment(relative.fragment()); + t.setFragment(relative.d->fragmentImpl()); removeDotsFromPath(&t.d->encodedPath); t.d->path.clear(); #if defined(QURL_DEBUG) qDebug("QUrl(\"%s\").resolved(\"%s\") = \"%s\"", - toEncoded().constData(), - relative.toEncoded().constData(), + d->toEncoded().constData(), + relative.d->toEncoded().constData(), t.toEncoded().constData()); #endif return t; @@ -5702,6 +5792,7 @@ QUrl QUrl::resolved(const QUrl &relative) const bool QUrl::isRelative() const { if (!d) return true; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); return d->scheme.isEmpty(); @@ -5717,13 +5808,14 @@ bool QUrl::isRelative() const QString QUrl::toString(FormattingOptions options) const { if (!d) return QString(); + QString ourPath = path(); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); QString url; if (!(options & QUrl::RemoveScheme) && !d->scheme.isEmpty()) url += d->scheme + QLatin1Char(':'); - QString ourPath = path(); if ((options & QUrl::RemoveAuthority) != QUrl::RemoveAuthority) { bool doFileScheme = d->scheme == QLatin1String("file") && ourPath.startsWith(QLatin1Char('/')); QString tmp = d->authority(options); @@ -5751,7 +5843,7 @@ QString QUrl::toString(FormattingOptions options) const } if (!(options & QUrl::RemoveFragment) && d->hasFragment) { url += QLatin1Char('#'); - url += fragment(); + url += d->fragmentImpl(); } return url; @@ -5769,6 +5861,7 @@ QString QUrl::toString(FormattingOptions options) const QByteArray QUrl::toEncoded(FormattingOptions options) const { if (!d) return QByteArray(); + QMutexLocker lock(&d->mutex); return d->toEncoded(options); } @@ -6022,6 +6115,7 @@ void QUrl::setIdnWhitelist(const QStringList &list) */ bool QUrl::operator <(const QUrl &url) const { + QOrderedMutexLocker(d ? &d->mutex : 0, url.d ? &url.d->mutex : 0); if (!d) return url.d ? QByteArray() < url.d->normalized() : false; if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); if (!url.d) return d->normalized() < QByteArray(); @@ -6037,6 +6131,7 @@ bool QUrl::operator ==(const QUrl &url) const { if (!d) return url.isEmpty(); if (!url.d) return isEmpty(); + QOrderedMutexLocker(&d->mutex, &url.d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); if (!QURL_HASFLAG(url.d->stateFlags, QUrlPrivate::Parsed)) url.d->parse(); return d->normalized() == url.d->normalized(); @@ -6099,10 +6194,39 @@ QUrl &QUrl::operator =(const QString &url) */ void QUrl::detach() { - if (!d) + if (!d) { d = new QUrlPrivate; - else - qAtomicDetach(d); + } else { + // Public method, so it must lock first. + QMutexLocker lock(&d->mutex); + detach(lock); + } +} + + +/*! \internal + + Forces a detach. Unlocks \a locker once the detaching is done. + + It's ok to access private members afterwards, without lock, since + after detaching we have our private copy of the d pointer, that + no other QUrl instance can know about yet. +*/ +void QUrl::detach(QMutexLocker &locker) +{ + Q_ASSERT(d); // we have a locker, so we have a d pointer + // Ensure that we hold the mutex until after making our private copy, + // so that another thread cannot deref + delete d meanwhile. + // So this is a modified version of qAtomicDetach(d) + if (d->ref == 1) { + locker.unlock(); + return; + } + QUrlPrivate * x = d; + d = new QUrlPrivate(*x); + locker.unlock(); + if (!x->ref.deref()) + delete x; } /*! @@ -6169,12 +6293,14 @@ QString QUrl::toLocalFile() const if (!d) return QString(); // the call to isLocalFile() also ensures that we're parsed - if (!isLocalFile() && !d->scheme.isEmpty()) + if (!isLocalFile() && !scheme().isEmpty()) return QString(); QString tmp; QString ourPath = path(); + QMutexLocker lock(&d->mutex); // for d->host + // magic for shared drive on windows if (!d->host.isEmpty()) { tmp = QLatin1String("//") + d->host + (ourPath.length() > 0 && ourPath.at(0) != QLatin1Char('/') @@ -6203,6 +6329,7 @@ QString QUrl::toLocalFile() const bool QUrl::isLocalFile() const { if (!d) return false; + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); if (d->scheme.compare(QLatin1String("file"), Qt::CaseInsensitive) != 0) @@ -6224,12 +6351,14 @@ bool QUrl::isParentOf(const QUrl &childUrl) const && (childUrl.authority().isEmpty()) && childPath.length() > 0 && childPath.at(0) == QLatin1Char('/')); + QMutexLocker lock(&d->mutex); if (!QURL_HASFLAG(d->stateFlags, QUrlPrivate::Parsed)) d->parse(); + lock.unlock(); QString ourPath = path(); - return ((childUrl.scheme().isEmpty() || d->scheme == childUrl.scheme()) - && (childUrl.authority().isEmpty() || d->authority() == childUrl.authority()) + return ((childUrl.scheme().isEmpty() || scheme() == childUrl.scheme()) + && (childUrl.authority().isEmpty() || authority() == childUrl.authority()) && childPath.startsWith(ourPath) && ((ourPath.endsWith(QLatin1Char('/')) && childPath.length() > ourPath.length()) || (!ourPath.endsWith(QLatin1Char('/')) @@ -6467,6 +6596,7 @@ QString QUrl::errorString() const { if (!d) return QLatin1String(QT_TRANSLATE_NOOP(QUrl, "Invalid URL \"\": ")); // XXX not a good message, but the one an empty URL produces + QMutexLocker lock(&d->mutex); return d->createErrorString(); } diff --git a/src/corelib/io/qurl.h b/src/corelib/io/qurl.h index 15d4174..d624418 100644 --- a/src/corelib/io/qurl.h +++ b/src/corelib/io/qurl.h @@ -56,6 +56,7 @@ QT_MODULE(Core) class QUrlPrivate; class QDataStream; +class QMutexLocker; class Q_CORE_EXPORT QUrl { @@ -274,6 +275,7 @@ protected: #endif private: + void detach(QMutexLocker &locker); QUrlPrivate *d; public: typedef QUrlPrivate * DataPtr; diff --git a/tests/auto/qurl/tst_qurl.cpp b/tests/auto/qurl/tst_qurl.cpp index 348ce44..38311c3 100644 --- a/tests/auto/qurl/tst_qurl.cpp +++ b/tests/auto/qurl/tst_qurl.cpp @@ -203,7 +203,11 @@ private slots: void taskQTBUG_8701(); void removeAllEncodedQueryItems_data(); void removeAllEncodedQueryItems(); + void detach(); + void testThreading(); +private: + void testThreadingHelper(); }; // Testing get/set functions @@ -271,6 +275,8 @@ void tst_QUrl::constructing() QVERIFY(url.isEmpty()); QCOMPARE(url.port(), -1); QCOMPARE(url.toString(), QString()); + QVERIFY(url == url); + QVERIFY(!(url < url)); QList > query; query += qMakePair(QString("type"), QString("login")); @@ -348,6 +354,8 @@ void tst_QUrl::comparison() QVERIFY(url2.isValid()); QVERIFY(url1 == url2); + QVERIFY(!(url1 < url2)); + QVERIFY(!(url2 < url1)); // 6.2.2 Syntax-based Normalization QUrl url3 = QUrl::fromEncoded("example://a/b/c/%7Bfoo%7D"); @@ -368,6 +376,7 @@ void tst_QUrl::comparison() QUrl url8; url8.setEncodedQuery("a=c"); QVERIFY(url7 != url8); + QVERIFY(url7 < url8); } void tst_QUrl::copying() @@ -4061,5 +4070,79 @@ void tst_QUrl::removeAllEncodedQueryItems() QCOMPARE(url, result); } +void tst_QUrl::detach() +{ + QUrl empty; + empty.detach(); + + QUrl foo("http://www.kde.org"); + QUrl foo2 = foo; + foo2.detach(); // not that it's needed, given that setHost detaches, of course. But this increases coverage :) + foo2.setHost("www.gnome.org"); + QCOMPARE(foo2.host(), QString("www.gnome.org")); + QCOMPARE(foo.host(), QString("www.kde.org")); +} + +// Test accessing the same QUrl from multiple threads concurrently +// To maximize the chances of a race (and of a report from helgrind), we actually use +// 10 urls rather than one. +class UrlStorage +{ +public: + UrlStorage() { + m_urls.resize(10); + for (int i = 0 ; i < m_urls.size(); ++i) + m_urls[i] = QUrl::fromEncoded("http://www.kde.org", QUrl::StrictMode); + } + QVector m_urls; +}; + +static const UrlStorage * s_urlStorage = 0; + +void tst_QUrl::testThreadingHelper() +{ + const UrlStorage* storage = s_urlStorage; + for (int i = 0 ; i < storage->m_urls.size(); ++i ) { + const QUrl& u = storage->m_urls.at(i); + // QVERIFY/QCOMPARE trigger race conditions in helgrind + if (!u.isValid()) + qFatal("invalid url"); + if (u.scheme() != QLatin1String("http")) + qFatal("invalid scheme"); + if (!u.toString().startsWith('h')) + qFatal("invalid toString"); + QUrl copy(u); + copy.setHost("www.new-host.com"); + QUrl copy2(u); + copy2.setUserName("dfaure"); + QUrl copy3(u); + copy3.setUrl("http://www.new-host.com"); + QUrl copy4(u); + copy4.detach(); + QUrl copy5(u); + QUrl resolved1 = u.resolved(QUrl("index.html")); + Q_UNUSED(resolved1); + QUrl resolved2 = QUrl("http://www.kde.org").resolved(u); + Q_UNUSED(resolved2); + QString local = u.toLocalFile(); + Q_UNUSED(local); + QTest::qWait(10); // give time for the other threads to start + } +} + +#include + +void tst_QUrl::testThreading() +{ + s_urlStorage = new UrlStorage; + QThreadPool::globalInstance()->setMaxThreadCount(100); + QFutureSynchronizer sync; + for (int i = 0; i < 100; ++i) + sync.addFuture(QtConcurrent::run(this, &tst_QUrl::testThreadingHelper)); + sync.waitForFinished(); + delete s_urlStorage; +} + QTEST_MAIN(tst_QUrl) + #include "tst_qurl.moc" -- cgit v0.12