Pull some processing out of environmentMutex critical sections

Move most string handling code out of the environmentMutex critical
sections to increase parallelism (Amdahl's Law).

I drew the line at temporarily dropping the mutex to allocate memory
for the return QByteArrays, as that would have meant to loop over
obtaining the size, allocating it, and then copying the data which
would have overly complicated the code.

Change-Id: I980cbf7fda76fc8057f84b2539b4a299e06cae82
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 0beb58cf58968acca84bf5f612288fe9c09e5879)
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

1 files changed, 12 insertions, 4 deletions

diff --git a/src/corelib/global/qglobal.cpp b/src/corelib/global/qglobal.cpp
index 259b92be83..d2805f52e1 100644
--- a/src/corelib/global/qglobal.cpp
+++ b/src/corelib/global/qglobal.cpp
@@ -3465,17 +3465,18 @@ QByteArray qgetenv(const char *varName)
 QString qEnvironmentVariable(const char *varName, const QString &defaultValue)
 {
 #if defined(Q_OS_WIN)
-    const auto locker = qt_scoped_lock(environmentMutex);
     QVarLengthArray<wchar_t, 32> wname(qsizetype(strlen(varName)) + 1);
     for (qsizetype i = 0; i < wname.size(); ++i) // wname.size() is correct: will copy terminating null
         wname[i] = uchar(varName[i]);
     size_t requiredSize = 0;
+    auto locker = qt_unique_lock(environmentMutex);
     _wgetenv_s(&requiredSize, 0, 0, wname.data());
     if (requiredSize == 0)
         return defaultValue;
     QString buffer(qsizetype(requiredSize), Qt::Uninitialized);
     _wgetenv_s(&requiredSize, reinterpret_cast<wchar_t *>(buffer.data()), requiredSize,
                wname.data());
+    locker.unlock();
     // requiredSize includes the terminating null, which we don't want.
     Q_ASSERT(buffer.endsWith(QChar(u'\0')));
     buffer.chop(1);
@@ -3618,18 +3619,22 @@ bool qEnvironmentVariableIsSet(const char *varName) noexcept
 */
 bool qputenv(const char *varName, const QByteArray &value)
 {
-    const auto locker = qt_scoped_lock(environmentMutex);
 #if defined(Q_CC_MSVC)
+    const auto locker = qt_scoped_lock(environmentMutex);
     return _putenv_s(varName, value.constData()) == 0;
 #elif (defined(_POSIX_VERSION) && (_POSIX_VERSION-0) >= 200112L) || defined(Q_OS_HAIKU)
     // POSIX.1-2001 has setenv
+    const auto locker = qt_scoped_lock(environmentMutex);
     return setenv(varName, value.constData(), true) == 0;
 #else
     QByteArray buffer(varName);
     buffer += '=';
     buffer += value;
     char *envVar = qstrdup(buffer.constData());
-    int result = putenv(envVar);
+    int result = [&] {
+        const auto locker = qt_scoped_lock(environmentMutex);
+        return putenv(envVar);
+    }();
     if (result != 0) // error. we have to delete the string.
         delete[] envVar;
     return result == 0;
@@ -3649,16 +3654,18 @@ bool qputenv(const char *varName, const QByteArray &value)
 */
 bool qunsetenv(const char *varName)
 {
-    const auto locker = qt_scoped_lock(environmentMutex);
 #if defined(Q_CC_MSVC)
+    const auto locker = qt_scoped_lock(environmentMutex);
     return _putenv_s(varName, "") == 0;
 #elif (defined(_POSIX_VERSION) && (_POSIX_VERSION-0) >= 200112L) || defined(Q_OS_BSD4) || defined(Q_OS_HAIKU)
     // POSIX.1-2001, BSD and Haiku have unsetenv
+    const auto locker = qt_scoped_lock(environmentMutex);
     return unsetenv(varName) == 0;
 #elif defined(Q_CC_MINGW)
     // On mingw, putenv("var=") removes "var" from the environment
     QByteArray buffer(varName);
     buffer += '=';
+    const auto locker = qt_scoped_lock(environmentMutex);
     return putenv(buffer.constData()) == 0;
 #else
     // Fallback to putenv("var=") which will insert an empty var into the
@@ -3666,6 +3673,7 @@ bool qunsetenv(const char *varName)
     QByteArray buffer(varName);
     buffer += '=';
     char *envVar = qstrdup(buffer.constData());
+    const auto locker = qt_scoped_lock(environmentMutex);
     return putenv(envVar) == 0;
 #endif
 }