lock baseEnv in cache()

as we modify the environment, it must be properly locked.
this implies that initFrom() also needs to be called with a lock.

Task-number: QTCREATORBUG-9835
Change-Id: I48bae9af9adaa0518e5a9db0ba08ff057ae14f9f
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

src/shared/proparser/qmakebuiltins.cpp

@@ -44,6 +44,9 @@
 #include <qset.h>
 #include <qstringlist.h>
 #include <qtextstream.h>
+#ifdef PROEVALUATOR_THREAD_SAFE
+# include <qthreadpool.h>
+#endif
 
 #ifdef Q_OS_UNIX
 #include <time.h>
@@ -1526,8 +1529,31 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateBuiltinConditional(
             ProStringList newval;
             bool changed = false;
             for (bool hostBuild = false; ; hostBuild = true) {
-                if (QMakeBaseEnv *baseEnv = m_option->baseEnvs.value(
-                            QMakeBaseKey(m_buildRoot, hostBuild))) {
+#ifdef PROEVALUATOR_THREAD_SAFE
+                m_option->mutex.lock();
+#endif
+                QMakeBaseEnv *baseEnv =
+                        m_option->baseEnvs.value(QMakeBaseKey(m_buildRoot, hostBuild));
+#ifdef PROEVALUATOR_THREAD_SAFE
+                // It's ok to unlock this before locking baseEnv,
+                // as we have no intention to initialize the env.
+                m_option->mutex.unlock();
+#endif
+                do {
+                    if (!baseEnv)
+                        break;
+#ifdef PROEVALUATOR_THREAD_SAFE
+                    QMutexLocker locker(&baseEnv->mutex);
+                    if (baseEnv->inProgress) {
+                        // The env is still in the works, but it may be already past the cache
+                        // loading. So we need to wait for completion and amend it as usual.
+                        QThreadPool::globalInstance()->releaseThread();
+                        baseEnv->cond.wait(&baseEnv->mutex);
+                        QThreadPool::globalInstance()->reserveThread();
+                    }
+                    if (!baseEnv->isOk)
+                        break;
+#endif
                     QMakeEvaluator *baseEval = baseEnv->evaluator;
                     const ProStringList &oldval = baseEval->values(dstvar);
                     if (mode == CacheSet) {
@@ -1558,7 +1584,7 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::evaluateBuiltinConditional(
                         }
                         changed = true;
                     }
-                }
+                } while (false);
                 if (hostBuild)
                     break;
             }

src/shared/proparser/qmakeevaluator.cpp

@@ -1302,47 +1302,45 @@ QMakeEvaluator::VisitReturn QMakeEvaluator::visitProFile(
         QMakeBaseEnv *baseEnv = *baseEnvPtr;
 
 #ifdef PROEVALUATOR_THREAD_SAFE
-        {
-            QMutexLocker locker(&baseEnv->mutex);
-            m_option->mutex.unlock();
-            if (baseEnv->inProgress) {
-                QThreadPool::globalInstance()->releaseThread();
-                baseEnv->cond.wait(&baseEnv->mutex);
-                QThreadPool::globalInstance()->reserveThread();
-                if (!baseEnv->isOk)
-                    return ReturnFalse;
-            } else
+        QMutexLocker locker(&baseEnv->mutex);
+        m_option->mutex.unlock();
+        if (baseEnv->inProgress) {
+            QThreadPool::globalInstance()->releaseThread();
+            baseEnv->cond.wait(&baseEnv->mutex);
+            QThreadPool::globalInstance()->reserveThread();
+            if (!baseEnv->isOk)
+                return ReturnFalse;
+        } else
 #endif
-            if (!baseEnv->evaluator) {
+        if (!baseEnv->evaluator) {
 #ifdef PROEVALUATOR_THREAD_SAFE
-                baseEnv->inProgress = true;
-                locker.unlock();
+            baseEnv->inProgress = true;
+            locker.unlock();
 #endif
 
-                QMakeEvaluator *baseEval = new QMakeEvaluator(m_option, m_parser, m_vfs, m_handler);
-                baseEnv->evaluator = baseEval;
-                baseEval->m_superfile = m_superfile;
-                baseEval->m_conffile = m_conffile;
-                baseEval->m_cachefile = m_cachefile;
-                baseEval->m_sourceRoot = m_sourceRoot;
-                baseEval->m_buildRoot = m_buildRoot;
-                baseEval->m_hostBuild = m_hostBuild;
-                bool ok = baseEval->loadSpec();
+            QMakeEvaluator *baseEval = new QMakeEvaluator(m_option, m_parser, m_vfs, m_handler);
+            baseEnv->evaluator = baseEval;
+            baseEval->m_superfile = m_superfile;
+            baseEval->m_conffile = m_conffile;
+            baseEval->m_cachefile = m_cachefile;
+            baseEval->m_sourceRoot = m_sourceRoot;
+            baseEval->m_buildRoot = m_buildRoot;
+            baseEval->m_hostBuild = m_hostBuild;
+            bool ok = baseEval->loadSpec();
 
 #ifdef PROEVALUATOR_THREAD_SAFE
-                locker.relock();
-                baseEnv->isOk = ok;
-                baseEnv->inProgress = false;
-                baseEnv->cond.wakeAll();
+            locker.relock();
+            baseEnv->isOk = ok;
+            baseEnv->inProgress = false;
+            baseEnv->cond.wakeAll();
 #endif
 
-                if (!ok)
-                    return ReturnFalse;
-            }
-#ifdef PROEVALUATOR_THREAD_SAFE
-            else if (!baseEnv->isOk)
+            if (!ok)
                 return ReturnFalse;
         }
+#ifdef PROEVALUATOR_THREAD_SAFE
+        else if (!baseEnv->isOk)
+            return ReturnFalse;
 #endif
 
         initFrom(*baseEnv->evaluator);