From 28ee554b37be39c03c231e7b857f71163dc6ea73 Mon Sep 17 00:00:00 2001 From: Joerg Bornemann Date: Tue, 30 Apr 2013 15:31:46 +0200 Subject: QProcess/Win: drain output pipes on process finish If a process dies before all output is read into the internal buffer of QProcess, we might lose data. Therefore we must drain the output pipes like we already do in the synchronous wait functions. Task-number: QTBUG-30843 Change-Id: I8bbc5265275c9ebd33218ba600267ae87d93ed61 Reviewed-by: Oswald Buddenhagen Reviewed-by: Friedemann Kleint --- src/corelib/io/qprocess.cpp | 1 + src/corelib/io/qprocess_p.h | 1 + src/corelib/io/qprocess_win.cpp | 22 +++++++++++----------- 3 files changed, 13 insertions(+), 11 deletions(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp index 59b6db7c79..3de1345585 100644 --- a/src/corelib/io/qprocess.cpp +++ b/src/corelib/io/qprocess.cpp @@ -1006,6 +1006,7 @@ bool QProcessPrivate::_q_processDied() return false; #endif #ifdef Q_OS_WIN + drainOutputPipes(); if (processFinishedNotifier) processFinishedNotifier->setEnabled(false); #endif diff --git a/src/corelib/io/qprocess_p.h b/src/corelib/io/qprocess_p.h index f658e54d4b..bd6943c8d0 100644 --- a/src/corelib/io/qprocess_p.h +++ b/src/corelib/io/qprocess_p.h @@ -317,6 +317,7 @@ public: bool waitForDeadChild(); #endif #ifdef Q_OS_WIN + bool drainOutputPipes(); void flushPipeWriter(); qint64 pipeWriterBytesToWrite() const; #endif diff --git a/src/corelib/io/qprocess_win.cpp b/src/corelib/io/qprocess_win.cpp index 0f36c3adbf..7776852277 100644 --- a/src/corelib/io/qprocess_win.cpp +++ b/src/corelib/io/qprocess_win.cpp @@ -625,21 +625,21 @@ bool QProcessPrivate::waitForStarted(int) return false; } -static bool drainOutputPipes(QProcessPrivate *d) +bool QProcessPrivate::drainOutputPipes() { - if (!d->stdoutReader && !d->stderrReader) + if (!stdoutReader && !stderrReader) return false; bool readyReadEmitted = false; forever { bool readOperationActive = false; - if (d->stdoutReader) { - readyReadEmitted |= d->stdoutReader->waitForReadyRead(0); - readOperationActive = d->stdoutReader->isReadOperationActive(); + if (stdoutReader) { + readyReadEmitted |= stdoutReader->waitForReadyRead(0); + readOperationActive = stdoutReader->isReadOperationActive(); } - if (d->stderrReader) { - readyReadEmitted |= d->stderrReader->waitForReadyRead(0); - readOperationActive |= d->stderrReader->isReadOperationActive(); + if (stderrReader) { + readyReadEmitted |= stderrReader->waitForReadyRead(0); + readOperationActive |= stderrReader->isReadOperationActive(); } if (!readOperationActive) break; @@ -669,7 +669,7 @@ bool QProcessPrivate::waitForReadyRead(int msecs) if (!pid) return false; if (WaitForSingleObject(pid->hProcess, 0) == WAIT_OBJECT_0) { - bool readyReadEmitted = drainOutputPipes(this); + bool readyReadEmitted = drainOutputPipes(); _q_processDied(); return readyReadEmitted; } @@ -772,12 +772,12 @@ bool QProcessPrivate::waitForFinished(int msecs) timer.resetIncrements(); if (!pid) { - drainOutputPipes(this); + drainOutputPipes(); return true; } if (WaitForSingleObject(pid->hProcess, timer.nextSleepTime()) == WAIT_OBJECT_0) { - drainOutputPipes(this); + drainOutputPipes(); _q_processDied(); return true; } -- cgit v1.2.3 From bbf1e1a66733dd95e34534bd6025fff8c0f3e4eb Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Fri, 11 Jan 2013 17:30:44 +0100 Subject: Add workaround for case-changing renaming of files on Linux/FAT32. The underlying rename() of the operating system simply does nothing when renaming 'foo' to 'Foo' in a case insensitive file system. Work around by moving in 2 steps. Change-Id: Ibc73724bfca402a5ce7fcf2a83e8fea32ff71093 Reviewed-by: Oswald Buddenhagen Reviewed-by: David Faure (KDE) Reviewed-by: Thiago Macieira --- src/corelib/io/qfile.cpp | 43 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 7 deletions(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp index afa62a075f..e46ba28f47 100644 --- a/src/corelib/io/qfile.cpp +++ b/src/corelib/io/qfile.cpp @@ -559,14 +559,43 @@ QFile::rename(const QString &newName) } // If the file exists and it is a case-changing rename ("foo" -> "Foo"), // compare Ids to make sure it really is a different file. - if (QFile::exists(newName) - && (d->fileName.compare(newName, Qt::CaseInsensitive) - || QFileSystemEngine::id(QFileSystemEntry(d->fileName)) != QFileSystemEngine::id(QFileSystemEntry(newName)))) { - // ### Race condition. If a file is moved in after this, it /will/ be - // overwritten. On Unix, the proper solution is to use hardlinks: - // return ::link(old, new) && ::remove(old); - d->setError(QFile::RenameError, tr("Destination file exists")); + if (QFile::exists(newName)) { + if (d->fileName.compare(newName, Qt::CaseInsensitive) + || QFileSystemEngine::id(QFileSystemEntry(d->fileName)) != QFileSystemEngine::id(QFileSystemEntry(newName))) { + // ### Race condition. If a file is moved in after this, it /will/ be + // overwritten. On Unix, the proper solution is to use hardlinks: + // return ::link(old, new) && ::remove(old); + d->setError(QFile::RenameError, tr("Destination file exists")); + return false; + } +#ifdef Q_OS_LINUX + // rename() on Linux simply does nothing when renaming "foo" to "Foo" on a case-insensitive + // FS, such as FAT32. Move the file away and rename in 2 steps to work around. + QTemporaryFile tempFile(d->fileName + QStringLiteral(".XXXXXX")); + tempFile.setAutoRemove(false); + if (!tempFile.open(QIODevice::ReadWrite)) { + d->setError(QFile::RenameError, tempFile.errorString()); + return false; + } + tempFile.close(); + if (!d->engine()->rename(tempFile.fileName())) { + d->setError(QFile::RenameError, tr("Error while renaming.")); + return false; + } + if (tempFile.rename(newName)) { + d->fileEngine->setFileName(newName); + d->fileName = newName; + return true; + } + d->setError(QFile::RenameError, tempFile.errorString()); + // We need to restore the original file. + if (!tempFile.rename(d->fileName)) { + d->setError(QFile::RenameError, errorString() + QLatin1Char('\n') + + tr("Unable to restore from %1: %2"). + arg(QDir::toNativeSeparators(tempFile.fileName()), tempFile.errorString())); + } return false; +#endif } unsetError(); close(); -- cgit v1.2.3 From 83315ce8b2b5c314fef2382f5ef5344de59fe3e3 Mon Sep 17 00:00:00 2001 From: Mitch Curtis Date: Mon, 22 Apr 2013 14:28:24 +0200 Subject: Revert QDateTime serialisation to pre-Qt 5 behaviour. In Qt 5, I managed to break the guarantee that a deserialised local datetime is the same time of day (potentially different UTC time), regardless of which timezone it was serialised in. This happened after I fixed QTBUG-4057 with If650e7960dca7b6ab44b8233410a6369c41df73a, which serialised datetimes as UTC. This patch reverts QDateTime serialisation to pre-Qt 5 behaviour to restore the guarantee and consequently re-opens QTBUG-4057. Change-Id: Iea877f7ed886f530b928067789b53534e89fe8cb Reviewed-by: Thiago Macieira --- src/corelib/io/qdatastream.cpp | 3 ++- src/corelib/io/qdatastream.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qdatastream.cpp b/src/corelib/io/qdatastream.cpp index dc7387bc85..b6926bc544 100644 --- a/src/corelib/io/qdatastream.cpp +++ b/src/corelib/io/qdatastream.cpp @@ -539,7 +539,7 @@ void QDataStream::setByteOrder(ByteOrder bo) \value Qt_4_8 Same as Qt_4_6. \value Qt_4_9 Same as Qt_4_6. \value Qt_5_0 Version 13 (Qt 5.0) - \value Qt_5_1 Same as Qt_5_0. + \value Qt_5_1 Version 14 (Qt 5.1) \sa setVersion(), version() */ @@ -571,6 +571,7 @@ void QDataStream::setByteOrder(ByteOrder bo) \table \header \li Qt Version \li QDataStream Version + \row \li Qt 5.1 \li 14 \row \li Qt 5.0 \li 13 \row \li Qt 4.6 \li 12 \row \li Qt 4.5 \li 11 diff --git a/src/corelib/io/qdatastream.h b/src/corelib/io/qdatastream.h index 6017ba5e7d..969cdf4517 100644 --- a/src/corelib/io/qdatastream.h +++ b/src/corelib/io/qdatastream.h @@ -86,7 +86,7 @@ public: Qt_4_8 = Qt_4_7, Qt_4_9 = Qt_4_8, Qt_5_0 = 13, - Qt_5_1 = Qt_5_0 + Qt_5_1 = 14 #if QT_VERSION >= 0x050200 #error Add the datastream version for this Qt version #endif -- cgit v1.2.3 From ab94a98b587580382fce420d8e78fa0dd3b06ff5 Mon Sep 17 00:00:00 2001 From: Kai Koehne Date: Mon, 22 Apr 2013 11:54:30 +0200 Subject: QUrl: Fix compiler warning about uninitialized var Fix gcc 4.8.0 warning about potential use of uninitialized variable. Change-Id: I0881b1209e9156323b2710c50256d4bed83930ca Reviewed-by: Thiago Macieira --- src/corelib/io/qurl.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qurl.cpp b/src/corelib/io/qurl.cpp index c71bb3afac..02e7b967ea 100644 --- a/src/corelib/io/qurl.cpp +++ b/src/corelib/io/qurl.cpp @@ -3731,7 +3731,7 @@ QString QUrl::errorString() const return QString(); QString errorSource; - int errorPosition; + int errorPosition = 0; QUrlPrivate::ErrorCode errorCode = d->validityError(&errorSource, &errorPosition); if (errorCode == QUrlPrivate::NoError) return QString(); -- cgit v1.2.3 From 305300cd9c7d6048c8a83312fcc6b9a3057e355a Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Sat, 27 Apr 2013 01:02:56 +0200 Subject: Avoid a QStringRef -> QString conversion by using the new qt_hash overload During qHash refactorings, this line was changed as qt_hash didn't have an overload taking a QStringRef. This causes a performance regression w.r.t. the same code in Qt 4. Task-number: QTBUG-30821 Change-Id: I17b27a54a73cb9061c20f1bd7f79d0c405050edd Reviewed-by: hjk --- src/corelib/io/qresource.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qresource.cpp b/src/corelib/io/qresource.cpp index 7dfc9b977c..04ec81e159 100644 --- a/src/corelib/io/qresource.cpp +++ b/src/corelib/io/qresource.cpp @@ -665,7 +665,7 @@ int QResourceRoot::findNode(const QString &_path, const QLocale &locale) const qDebug() << " " << child+j << " :: " << name(child+j); } #endif - const uint h = qt_hash(segment.toString()); + const uint h = qt_hash(segment); //do the binary search for the hash int l = 0, r = child_count-1; -- cgit v1.2.3 From f7eea69a2a322e156594cf5608bbbb97b4d364c1 Mon Sep 17 00:00:00 2001 From: Jake Petroules Date: Wed, 8 May 2013 08:25:15 -0400 Subject: Utilize the new Q_OS_MACX define. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All occurrences of `#if defined(Q_OS_MAC) && !defined(Q_OS_IOS)` have been replaced with `#if defined(Q_OS_MACX)`. Change-Id: I5055d9bd1845136beb8ed1c79a8f0f2c0897751a Reviewed-by: Oswald Buddenhagen Reviewed-by: Tor Arne Vestbø Reviewed-by: Thiago Macieira --- src/corelib/io/qfilesystemengine.cpp | 2 +- src/corelib/io/qfilesystemengine_p.h | 2 +- src/corelib/io/qfilesystemengine_unix.cpp | 16 ++++++++-------- src/corelib/io/qfilesystemmetadata_p.h | 4 ++-- src/corelib/io/qprocess.cpp | 2 +- src/corelib/io/qprocess_unix.cpp | 2 +- 6 files changed, 14 insertions(+), 14 deletions(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qfilesystemengine.cpp b/src/corelib/io/qfilesystemengine.cpp index 71ecc4a0a0..53cf6158ad 100644 --- a/src/corelib/io/qfilesystemengine.cpp +++ b/src/corelib/io/qfilesystemengine.cpp @@ -272,7 +272,7 @@ void QFileSystemMetaData::fillFromStatBuf(const QT_STATBUF &statBuffer) // Attributes entryFlags |= QFileSystemMetaData::ExistsAttribute; size_ = statBuffer.st_size; -#if defined (Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) if (statBuffer.st_flags & UF_HIDDEN) { entryFlags |= QFileSystemMetaData::HiddenAttribute; knownFlagsMask |= QFileSystemMetaData::HiddenAttribute; diff --git a/src/corelib/io/qfilesystemengine_p.h b/src/corelib/io/qfilesystemengine_p.h index f3d3018da6..5719629958 100644 --- a/src/corelib/io/qfilesystemengine_p.h +++ b/src/corelib/io/qfilesystemengine_p.h @@ -84,7 +84,7 @@ public: static QString resolveGroupName(uint groupId); #endif -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) static QString bundleName(const QFileSystemEntry &entry); #else static QString bundleName(const QFileSystemEntry &entry) { Q_UNUSED(entry) return QString(); } diff --git a/src/corelib/io/qfilesystemengine_unix.cpp b/src/corelib/io/qfilesystemengine_unix.cpp index 4bfb03a41e..b18e32b29a 100644 --- a/src/corelib/io/qfilesystemengine_unix.cpp +++ b/src/corelib/io/qfilesystemengine_unix.cpp @@ -59,7 +59,7 @@ QT_BEGIN_NAMESPACE -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) static inline bool _q_isMacHidden(const char *nativePath) { OSErr err; @@ -143,7 +143,7 @@ QFileSystemEntry QFileSystemEngine::getLinkTarget(const QFileSystemEntry &link, ret.chop(1); return QFileSystemEntry(ret); } -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) { FSRef fref; if (FSPathMakeRef((const UInt8 *)QFile::encodeName(QDir::cleanPath(link.filePath())).data(), &fref, 0) == noErr) { @@ -175,7 +175,7 @@ QFileSystemEntry QFileSystemEngine::canonicalName(const QFileSystemEntry &entry, return QFileSystemEntry(slowCanonicalized(absoluteName(entry).filePath())); #else char *ret = 0; -# if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +# if defined(Q_OS_MACX) // When using -mmacosx-version-min=10.4, we get the legacy realpath implementation, // which does not work properly with the realpath(X,0) form. See QTBUG-28282. if (QSysInfo::MacintoshVersion >= QSysInfo::MV_10_6) { @@ -335,7 +335,7 @@ QString QFileSystemEngine::resolveGroupName(uint groupId) return QString(); } -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) //static QString QFileSystemEngine::bundleName(const QFileSystemEntry &entry) { @@ -355,7 +355,7 @@ QString QFileSystemEngine::bundleName(const QFileSystemEntry &entry) bool QFileSystemEngine::fillMetaData(const QFileSystemEntry &entry, QFileSystemMetaData &data, QFileSystemMetaData::MetaDataFlags what) { -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) if (what & QFileSystemMetaData::BundleType) { if (!data.hasFlags(QFileSystemMetaData::DirectoryType)) what |= QFileSystemMetaData::DirectoryType; @@ -364,7 +364,7 @@ bool QFileSystemEngine::fillMetaData(const QFileSystemEntry &entry, QFileSystemM // Mac OS >= 10.5: st_flags & UF_HIDDEN what |= QFileSystemMetaData::PosixStatFlags; } -#endif // defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#endif // defined(Q_OS_MACX) if (what & QFileSystemMetaData::PosixStatFlags) what |= QFileSystemMetaData::PosixStatFlags; @@ -425,7 +425,7 @@ bool QFileSystemEngine::fillMetaData(const QFileSystemEntry &entry, QFileSystemM | QFileSystemMetaData::ExistsAttribute; } -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) if (what & QFileSystemMetaData::AliasType) { if (entryExists) { @@ -471,7 +471,7 @@ bool QFileSystemEngine::fillMetaData(const QFileSystemEntry &entry, QFileSystemM data.knownFlagsMask |= QFileSystemMetaData::HiddenAttribute; } -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) if (what & QFileSystemMetaData::BundleType) { if (entryExists && data.isDirectory()) { QCFType path = CFStringCreateWithBytes(0, diff --git a/src/corelib/io/qfilesystemmetadata_p.h b/src/corelib/io/qfilesystemmetadata_p.h index 2444a5fd63..1abc9b7ec4 100644 --- a/src/corelib/io/qfilesystemmetadata_p.h +++ b/src/corelib/io/qfilesystemmetadata_p.h @@ -100,7 +100,7 @@ public: LinkType = 0x00010000, FileType = 0x00020000, DirectoryType = 0x00040000, -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) BundleType = 0x00080000, AliasType = 0x08000000, #else @@ -248,7 +248,7 @@ private: Q_DECLARE_OPERATORS_FOR_FLAGS(QFileSystemMetaData::MetaDataFlags) -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) inline bool QFileSystemMetaData::isBundle() const { return (entryFlags & BundleType); } inline bool QFileSystemMetaData::isAlias() const { return (entryFlags & AliasType); } #else diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp index 3de1345585..6ff43ab943 100644 --- a/src/corelib/io/qprocess.cpp +++ b/src/corelib/io/qprocess.cpp @@ -2340,7 +2340,7 @@ bool QProcess::startDetached(const QString &program) } QT_BEGIN_INCLUDE_NAMESPACE -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) # include # define environ (*_NSGetEnviron()) #elif defined(Q_OS_WINCE) || defined(Q_OS_IOS) diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp index 7f7066271b..79a75b0321 100644 --- a/src/corelib/io/qprocess_unix.cpp +++ b/src/corelib/io/qprocess_unix.cpp @@ -472,7 +472,7 @@ bool QProcessPrivate::createChannel(Channel &channel) } QT_BEGIN_INCLUDE_NAMESPACE -#if defined(Q_OS_MAC) && !defined(Q_OS_IOS) +#if defined(Q_OS_MACX) # include # define environ (*_NSGetEnviron()) #else -- cgit v1.2.3 From 95cab90b1023a824017b6d22f62987ba9742b6b3 Mon Sep 17 00:00:00 2001 From: Albert Astals Cid Date: Wed, 8 May 2013 18:02:02 +0200 Subject: Make QFlags enum flags (C++11 strict enums) friendly Change-Id: I9ccb3e4d281a545ca1845db4f6aa7ac6c04e8621 Reviewed-by: Olivier Goffart --- src/corelib/io/qdebug.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qdebug.h b/src/corelib/io/qdebug.h index ce72fcd26d..9ed5f6e951 100644 --- a/src/corelib/io/qdebug.h +++ b/src/corelib/io/qdebug.h @@ -280,7 +280,7 @@ inline QDebug operator<<(QDebug debug, const QFlags &flags) debug.nospace() << '|'; else needSeparator = true; - debug.nospace() << "0x" << QByteArray::number(T(1 << i), 16).constData(); + debug.nospace() << "0x" << QByteArray::number(typename QFlags::Int(1) << i, 16).constData(); } } debug << ')'; -- cgit v1.2.3 From ea461b7800a5d33e570329bedd9d9340d4577df5 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sat, 4 May 2013 08:33:09 -0700 Subject: QProcess: leave a warning about the signal with overload, for Qt 6 You can't write: connect(proc, &QProcess::finished, [](){}); because of the overload. Change-Id: I651cc56ee15481392590dc44942d8e814fad75f6 Reviewed-by: Olivier Goffart Reviewed-by: Oswald Buddenhagen --- src/corelib/io/qprocess.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qprocess.h b/src/corelib/io/qprocess.h index 29adf37f74..9da3e63f38 100644 --- a/src/corelib/io/qprocess.h +++ b/src/corelib/io/qprocess.h @@ -219,7 +219,7 @@ Q_SIGNALS: QPrivateSignal #endif ); - void finished(int exitCode); + void finished(int exitCode); // ### Qt 6: merge the two signals with a default value void finished(int exitCode, QProcess::ExitStatus exitStatus); void error(QProcess::ProcessError error); void stateChanged(QProcess::ProcessState state -- cgit v1.2.3 From 3f1519d40a118c8519c910e4d138829d36e16690 Mon Sep 17 00:00:00 2001 From: Bjoern Breitmeyer Date: Mon, 13 May 2013 18:10:43 +0200 Subject: Fixed WINCE linking error in QProcess The error was introduced with 28ee554b37be39c03c231e7b857f71163dc6ea73 Change-Id: If3e51227af3880496ef2da3a18835b36d65bad78 Reviewed-by: Friedemann Kleint Reviewed-by: Oswald Buddenhagen --- src/corelib/io/qprocess_wince.cpp | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qprocess_wince.cpp b/src/corelib/io/qprocess_wince.cpp index 712748aa59..ad9a328133 100644 --- a/src/corelib/io/qprocess_wince.cpp +++ b/src/corelib/io/qprocess_wince.cpp @@ -233,6 +233,11 @@ bool QProcessPrivate::waitForStarted(int) return false; } +bool QProcessPrivate::drainOutputPipes() +{ + return true; +} + bool QProcessPrivate::waitForReadyRead(int msecs) { return false; -- cgit v1.2.3 From 85e61297f7b02297641826332dbdbc845a88c34b Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 19 Apr 2013 18:11:05 +0200 Subject: restore QProcessEnvironment shared data thread safety on unix implicit sharing together with 'mutable' is a time bomb. we need to protect the nameMap, because concurrent "reads" may try to insert into the hash, which would go boom. we need to protect the key/value of Hash objects, because while the refcounting is atomic, the d pointer assignments are not, which would also go boom. we can simply use a QMutex to protect the whole environment, because it is very cheap in the uncontended case. Task-number: QTBUG-30779 Change-Id: Iaad5720041ca06691d75eb9c6c0e1c120d4a7b46 Reviewed-by: Olivier Goffart --- src/corelib/io/qprocess.cpp | 35 +++++++++++++++++++++++++------- src/corelib/io/qprocess_p.h | 43 ++++++++++++++++++++++++++++++++++++++++ src/corelib/io/qprocess_unix.cpp | 4 +++- 3 files changed, 74 insertions(+), 8 deletions(-) (limited to 'src/corelib/io') diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp index 6ff43ab943..b1861d8038 100644 --- a/src/corelib/io/qprocess.cpp +++ b/src/corelib/io/qprocess.cpp @@ -265,7 +265,13 @@ QProcessEnvironment &QProcessEnvironment::operator=(const QProcessEnvironment &o */ bool QProcessEnvironment::operator==(const QProcessEnvironment &other) const { - return d == other.d || (d && other.d && d->hash == other.d->hash); + if (d == other.d) + return true; + if (d && other.d) { + QProcessEnvironmentPrivate::OrderedMutexLocker locker(d, other.d); + return d->hash == other.d->hash; + } + return false; } /*! @@ -276,6 +282,7 @@ bool QProcessEnvironment::operator==(const QProcessEnvironment &other) const */ bool QProcessEnvironment::isEmpty() const { + // Needs no locking, as no hash nodes are accessed return d ? d->hash.isEmpty() : true; } @@ -302,7 +309,10 @@ void QProcessEnvironment::clear() */ bool QProcessEnvironment::contains(const QString &name) const { - return d ? d->hash.contains(d->prepareName(name)) : false; + if (!d) + return false; + QProcessEnvironmentPrivate::MutexLocker locker(d); + return d->hash.contains(d->prepareName(name)); } /*! @@ -319,7 +329,8 @@ bool QProcessEnvironment::contains(const QString &name) const */ void QProcessEnvironment::insert(const QString &name, const QString &value) { - // d detaches from null + // our re-impl of detach() detaches from null + d.detach(); // detach before prepareName() d->hash.insert(d->prepareName(name), d->prepareValue(value)); } @@ -333,8 +344,10 @@ void QProcessEnvironment::insert(const QString &name, const QString &value) */ void QProcessEnvironment::remove(const QString &name) { - if (d) + if (d) { + d.detach(); // detach before prepareName() d->hash.remove(d->prepareName(name)); + } } /*! @@ -349,6 +362,7 @@ QString QProcessEnvironment::value(const QString &name, const QString &defaultVa if (!d) return defaultValue; + QProcessEnvironmentPrivate::MutexLocker locker(d); QProcessEnvironmentPrivate::Hash::ConstIterator it = d->hash.constFind(d->prepareName(name)); if (it == d->hash.constEnd()) return defaultValue; @@ -371,7 +385,10 @@ QString QProcessEnvironment::value(const QString &name, const QString &defaultVa */ QStringList QProcessEnvironment::toStringList() const { - return d ? d->toList() : QStringList(); + if (!d) + return QStringList(); + QProcessEnvironmentPrivate::MutexLocker locker(d); + return d->toList(); } /*! @@ -382,7 +399,10 @@ QStringList QProcessEnvironment::toStringList() const */ QStringList QProcessEnvironment::keys() const { - return d ? d->keys() : QStringList(); + if (!d) + return QStringList(); + QProcessEnvironmentPrivate::MutexLocker locker(d); + return d->keys(); } /*! @@ -397,7 +417,8 @@ void QProcessEnvironment::insert(const QProcessEnvironment &e) if (!e.d) return; - // d detaches from null + // our re-impl of detach() detaches from null + QProcessEnvironmentPrivate::MutexLocker locker(e.d); d->insert(*e.d); } diff --git a/src/corelib/io/qprocess_p.h b/src/corelib/io/qprocess_p.h index bd6943c8d0..2a2cc9fb84 100644 --- a/src/corelib/io/qprocess_p.h +++ b/src/corelib/io/qprocess_p.h @@ -59,6 +59,9 @@ #include "QtCore/qshareddata.h" #include "private/qringbuffer_p.h" #include "private/qiodevice_p.h" +#ifdef Q_OS_UNIX +#include +#endif #ifdef Q_OS_WIN #include "QtCore/qt_windows.h" @@ -148,6 +151,13 @@ public: inline QString nameToString(const Key &name) const { return name; } inline Value prepareValue(const QString &value) const { return value; } inline QString valueToString(const Value &value) const { return value; } + struct MutexLocker { + MutexLocker(const QProcessEnvironmentPrivate *) {} + }; + struct OrderedMutexLocker { + OrderedMutexLocker(const QProcessEnvironmentPrivate *, + const QProcessEnvironmentPrivate *) {} + }; #else inline Key prepareName(const QString &name) const { @@ -164,6 +174,37 @@ public: } inline Value prepareValue(const QString &value) const { return Value(value); } inline QString valueToString(const Value &value) const { return value.string(); } + + struct MutexLocker : public QMutexLocker + { + MutexLocker(const QProcessEnvironmentPrivate *d) : QMutexLocker(&d->mutex) {} + }; + struct OrderedMutexLocker : public QOrderedMutexLocker + { + OrderedMutexLocker(const QProcessEnvironmentPrivate *d1, + const QProcessEnvironmentPrivate *d2) : + QOrderedMutexLocker(&d1->mutex, &d2->mutex) + {} + }; + + QProcessEnvironmentPrivate() : QSharedData() {} + QProcessEnvironmentPrivate(const QProcessEnvironmentPrivate &other) : + QSharedData() + { + // This being locked ensures that the functions that only assign + // d pointers don't need explicit locking. + // We don't need to lock our own mutex, as this object is new and + // consequently not shared. For the same reason, non-const methods + // do not need a lock, as they detach objects (however, we need to + // ensure that they really detach before using prepareName()). + MutexLocker locker(&other); + hash = other.hash; + nameMap = other.nameMap; + // We need to detach our members, so that our mutex can protect them. + // As we are being detached, they likely would be detached a moment later anyway. + hash.detach(); + nameMap.detach(); + } #endif typedef QHash Hash; @@ -172,6 +213,8 @@ public: #ifdef Q_OS_UNIX typedef QHash NameHash; mutable NameHash nameMap; + + mutable QMutex mutex; #endif static QProcessEnvironment fromList(const QStringList &list); diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp index 79a75b0321..e9957d2384 100644 --- a/src/corelib/io/qprocess_unix.cpp +++ b/src/corelib/io/qprocess_unix.cpp @@ -617,8 +617,10 @@ void QProcessPrivate::startProcess() // Duplicate the environment. int envc = 0; char **envp = 0; - if (environment.d.constData()) + if (environment.d.constData()) { + QProcessEnvironmentPrivate::MutexLocker locker(environment.d); envp = _q_dupEnvironment(environment.d.constData()->hash, &envc); + } // Encode the working directory if it's non-empty, otherwise just pass 0. const char *workingDirPtr = 0; -- cgit v1.2.3