Fix potential crash in project setup.

If we
    - held an existing project open (in an IDE) and
    - changed its build directory and
    - a second qbs instance also held a project open there and
    - that second qbs instance used the same profile
then we would correctly detect that a competing build graph was already
present, but afterwards we would erroneously delete the build graph lock
object of the existing project, leading to a double-delete later when
the project itself was deleted.

Task-number: QTCREATORBUG-16376
Change-Id: Ie6c621f1dab5cc7b7ff97bf6c25d62609dc9eb35
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

2 files changed, 37 insertions, 1 deletions

diff --git a/src/lib/corelib/api/internaljobs.cpp b/src/lib/corelib/api/internaljobs.cpp
index 2c0693566..87ae69617 100644
--- a/src/lib/corelib/api/internaljobs.cpp
+++ b/src/lib/corelib/api/internaljobs.cpp
@@ -226,6 +226,7 @@ TopLevelProjectPtr InternalSetupProjectJob::project() const
 
 void InternalSetupProjectJob::start()
 {
+    const TopLevelProjectConstPtr existingProject = m_existingProject;
     BuildGraphLocker *bgLocker = m_existingProject ? m_existingProject->bgLocker : 0;
     try {
         const ErrorInfo err = m_parameters.expandBuildConfiguration();
@@ -249,7 +250,9 @@ void InternalSetupProjectJob::start()
     } catch (const ErrorInfo &error) {
         m_newProject.clear();
         setError(error);
-        if (!m_existingProject)
+
+        // Delete the build graph locker if and only if we allocated it here.
+        if (!existingProject)
             delete bgLocker;
     }
     emit finished(this);
diff --git a/tests/auto/api/tst_api.cpp b/tests/auto/api/tst_api.cpp
index be082a260..5ba952490 100644
--- a/tests/auto/api/tst_api.cpp
+++ b/tests/auto/api/tst_api.cpp
@@ -253,11 +253,44 @@ void TestApi::buildGraphLocking()
     QVERIFY2(!setupJob->error().hasError(), qPrintable(setupJob->error().toString()));
     const qbs::Project project = setupJob->project();
     Q_UNUSED(project);
+
+    // Case 1: Setting up a competing project from scratch.
     setupJob.reset(qbs::Project().setupProject(setupParams, m_logSink, 0));
     waitForFinished(setupJob.data());
     QVERIFY(setupJob->error().hasError());
     QVERIFY2(setupJob->error().toString().contains("lock"),
              qPrintable(setupJob->error().toString()));
+
+    // Case 2: Setting up a non-competing project and then making it competing.
+    qbs::SetupProjectParameters setupParams2 = setupParams;
+    setupParams2.setBuildRoot(setupParams.buildRoot() + "/2");
+    setupJob.reset(qbs::Project().setupProject(setupParams2, m_logSink, 0));
+    waitForFinished(setupJob.data());
+    QVERIFY2(!setupJob->error().hasError(), qPrintable(setupJob->error().toString()));
+    const QString buildDirName = profileName() + '-' + setupParams2.buildVariant();
+    const QString lockFile = setupParams2.buildRoot() + '/' + buildDirName + '/' + buildDirName
+            + ".bg.lock";
+    QVERIFY2(QFileInfo(lockFile).isFile(), qPrintable(lockFile));
+    qbs::Project project2 = setupJob->project();
+    QVERIFY(project2.isValid());
+    setupJob.reset(project2.setupProject(setupParams, m_logSink, 0));
+    waitForFinished(setupJob.data());
+    QVERIFY(setupJob->error().hasError());
+    QVERIFY2(setupJob->error().toString().contains("lock"),
+             qPrintable(setupJob->error().toString()));
+    QVERIFY2(QFileInfo(lockFile).isFile(), qPrintable(lockFile));
+
+    // Case 3: Changing the build directory of an existing project to something con-competing.
+    qbs::SetupProjectParameters setupParams3 = setupParams2;
+    setupParams3.setBuildRoot(setupParams.buildRoot() + "/3");
+    setupJob.reset(qbs::Project().setupProject(setupParams3, m_logSink, 0));
+    waitForFinished(setupJob.data());
+    QVERIFY2(!setupJob->error().hasError(), qPrintable(setupJob->error().toString()));
+    project2 = qbs::Project();
+    QVERIFY2(!QFileInfo(lockFile).exists(), qPrintable(lockFile));
+    const QString newLockFile = setupParams3.buildRoot() + '/' + buildDirName + '/'
+            + buildDirName + ".bg.lock";
+    QVERIFY2(QFileInfo(newLockFile).isFile(), qPrintable(newLockFile));
 }
 
 void TestApi::buildProject()