Qt4Project: Clean up DebugHelperBuildTask

Make sure that the QtVersion object isn't referenced any more in
the separate thread: It might be deleted / altered while the thread is
running. Also let QmlDump use the class, instead of relying on it's
own implementation.

Reviewed-by: ckamm

src/plugins/qt4projectmanager/debugginghelperbuildtask.cpp

@@ -32,13 +32,73 @@
 **************************************************************************/
 
 #include "debugginghelperbuildtask.h"
+#include "qmldumptool.h"
+#include "qmlobservertool.h"
+#include <projectexplorer/debugginghelper.h>
+#include <QCoreApplication>
 
 using namespace Qt4ProjectManager;
 using namespace Qt4ProjectManager::Internal;
+using ProjectExplorer::DebuggingHelperLibrary;
 
-DebuggingHelperBuildTask::DebuggingHelperBuildTask(const QSharedPointer<QtVersion> &version) :
-    m_version(version)
+
+DebuggingHelperBuildTask::DebuggingHelperBuildTask(QtVersion *version, DebuggingHelperTools tools)
 {
+    //
+    // Extract all information we need from version, such that we don't depend on the existence
+    // of the version pointer while compiling
+    //
+    m_qtId = version->uniqueId();
+    m_qtInstallData = version->versionInfo().value("QT_INSTALL_DATA");
+    if (m_qtInstallData.isEmpty()) {
+        m_errorMessage
+                = QCoreApplication::translate(
+                    "QtVersion",
+                    "Cannot determine the installation path for Qt version '%1'."
+                    ).arg(version->displayName());
+        return;
+    }
+
+    m_environment = Utils::Environment::systemEnvironment();
+    version->addToEnvironment(m_environment);
+
+    // TODO: the debugging helper doesn't comply to actual tool chain yet
+    ProjectExplorer::ToolChain *tc = 0;
+    foreach (ProjectExplorer::ToolChainType toolChainType, version->possibleToolChainTypes()) {
+        tc = version->toolChain(toolChainType);
+        if (tc)
+            break;
+    }
+
+    if (!tc) {
+        m_errorMessage =
+                QCoreApplication::translate(
+                    "QtVersion",
+                    "The Qt Version has no toolchain.");
+        return;
+    }
+
+    tc->addToEnvironment(m_environment);
+
+    m_target = (tc->type() == ProjectExplorer::ToolChain_GCC_MAEMO ? QLatin1String("-unix")
+                                                                   : QLatin1String(""));
+    m_qmakeCommand = version->qmakeCommand();
+    m_makeCommand = tc->makeCommand();
+    m_mkspec = version->mkspec();
+
+    m_tools = tools;
+
+    // Check the build requirements of the tools
+    if (m_tools & QmlDump) {
+        if (!QmlDumpTool::canBuild(version)) {
+            m_tools ^= QmlDump;
+        }
+    }
+    if (m_tools & QmlObserver) {
+        if (!QmlObserverTool::canBuild(version)) {
+            m_tools ^= QmlObserver;
+        }
+    }
 }
 
 DebuggingHelperBuildTask::~DebuggingHelperBuildTask()
@@ -49,13 +109,56 @@ void DebuggingHelperBuildTask::run(QFutureInterface<void> &future)
 {
     future.setProgressRange(0, 5);
     future.setProgressValue(1);
+
     QString output;
-    QString errorMessage;
-    if (m_version->buildDebuggingHelperLibrary(future, false, &output, &errorMessage)) {
-        emit finished(m_version->displayName(), output);
+
+    bool success = false;
+    if (m_errorMessage.isEmpty()) // might be already set in constructor
+        success = buildDebuggingHelper(future, &output);
+
+    if (success) {
+        emit finished(m_qtId, output);
     } else {
-        qWarning("%s", qPrintable(errorMessage));
-        emit finished(m_version->displayName(), errorMessage);
+        qWarning("%s", qPrintable(m_errorMessage));
+        emit finished(m_qtId, m_errorMessage);
     }
+
     deleteLater();
 }
+
+bool DebuggingHelperBuildTask::buildDebuggingHelper(QFutureInterface<void> &future, QString *output)
+{
+    if (m_tools & GdbDebugging) {
+        const QString gdbHelperDirectory = DebuggingHelperLibrary::copy(m_qtInstallData,
+                                                                        &m_errorMessage);
+        if (gdbHelperDirectory.isEmpty())
+            return false;
+        if (!DebuggingHelperLibrary::build(gdbHelperDirectory, m_makeCommand,
+                                           m_qmakeCommand, m_mkspec, m_environment,
+                                           m_target, output, &m_errorMessage))
+            return false;
+    }
+    future.setProgressValue(2);
+
+    if (m_tools & QmlDump) {
+        const QString qmlDumpToolDirectory = QmlDumpTool::copy(m_qtInstallData, &m_errorMessage);
+        if (qmlDumpToolDirectory.isEmpty())
+            return false;
+        if (!QmlDumpTool::build(qmlDumpToolDirectory, m_makeCommand, m_qmakeCommand, m_mkspec,
+                                m_environment, m_target, output, &m_errorMessage))
+            return false;
+    }
+    future.setProgressValue(3);
+
+    if (m_tools & QmlObserver) {
+        const QString qmlObserverDirectory = QmlObserverTool::copy(m_qtInstallData,
+                                                                   &m_errorMessage);
+        if (qmlObserverDirectory.isEmpty())
+            return false;
+        if (!QmlObserverTool::build(qmlObserverDirectory, m_makeCommand, m_qmakeCommand, m_mkspec,
+                                    m_environment, m_target, output, &m_errorMessage))
+            return false;
+    }
+    future.setProgressValue(4);
+    return true;
+}

src/plugins/qt4projectmanager/debugginghelperbuildtask.h

@@ -41,23 +41,39 @@
 namespace Qt4ProjectManager {
 namespace Internal {
 
-// A task suitable to be run by QtConcurrent to build the helpers.
-// Note that it may outlive the settings page if someone quickly cancels it,
-// so, the versions are passed around by QSharedPointer.
 class DebuggingHelperBuildTask : public QObject {
     Q_DISABLE_COPY(DebuggingHelperBuildTask)
     Q_OBJECT
 public:
-    explicit DebuggingHelperBuildTask(const QSharedPointer<QtVersion> &version);
+    enum DebuggingHelper {
+        GdbDebugging = 0x01,
+        QmlObserver = 0x02,
+        QmlDump = 0x04,
+        AllTools = GdbDebugging | QmlObserver | QmlDump
+    };
+    Q_DECLARE_FLAGS(DebuggingHelperTools, DebuggingHelper)
+
+    explicit DebuggingHelperBuildTask(QtVersion *version, DebuggingHelperTools tools = AllTools);
     virtual ~DebuggingHelperBuildTask();
 
     void run(QFutureInterface<void> &future);
 
 signals:
-    void finished(const QString &versionName, const QString &output);
+    void finished(int qtVersionId, const QString &output);
 
 private:
-    QSharedPointer<QtVersion> m_version;
+    bool buildDebuggingHelper(QFutureInterface<void> &future, QString *output);
+
+    DebuggingHelperTools m_tools;
+
+    int m_qtId;
+    QString m_qtInstallData;
+    QString m_target;
+    QString m_qmakeCommand;
+    QString m_makeCommand;
+    QString m_mkspec;
+    Utils::Environment m_environment;
+    QString m_errorMessage;
 };
 
 } //namespace Internal

src/plugins/qt4projectmanager/qmldumptool.cpp

@@ -35,6 +35,7 @@
 #include "qt4project.h"
 #include "qt4projectmanagerconstants.h"
 #include "qtversionmanager.h"
+#include "debugginghelperbuildtask.h"
 #include <coreplugin/icore.h>
 #include <coreplugin/progressmanager/progressmanager.h>
 
@@ -51,7 +52,10 @@
 #include <QHash>
 
 namespace {
+
 using namespace Qt4ProjectManager;
+using Qt4ProjectManager::Internal::DebuggingHelperBuildTask;
+
 
 class QmlDumpBuildTask;
 
@@ -64,33 +68,18 @@ class QmlDumpBuildTask : public QObject {
     Q_OBJECT
 public:
     explicit QmlDumpBuildTask(QtVersion *version)
-        : m_version(*version)
+        : m_buildTask(new DebuggingHelperBuildTask(version, DebuggingHelperBuildTask::QmlDump))
         , m_failed(false)
     {
         qmlDumpBuilds()->insert(m_version.uniqueId(), this);
+
+        connect(m_buildTask, SIGNAL(finished(int,QString)), this, SLOT(finish(int,QString)),
+                Qt::QueuedConnection);
     }
 
     void run(QFutureInterface<void> &future)
     {
-        future.setProgressRange(0, 5);
-        future.setProgressValue(1);
-        QString output;
-        QString errorMessage;
-        QString path;
-        if (m_version.buildDebuggingHelperLibrary(future, true, &output, &errorMessage)) {
-            const QString qtInstallData = m_version.versionInfo().value("QT_INSTALL_DATA");
-            path = QmlDumpTool::toolByInstallData(qtInstallData);
-            if (path.isEmpty())
-                errorMessage = QString::fromLatin1("Could not build QML plugin dumping helper for %1\nOutput:\n%2").
-                               arg(m_version.displayName(), output);
-        }
-        m_failed = path.isEmpty();
-        if (m_failed) {
-            qWarning("%s", qPrintable(errorMessage));
-        } else {
-            // proceed in gui thread
-            metaObject()->invokeMethod(this, "finish", Qt::QueuedConnection, Q_ARG(QString, path));
-        }
+        m_buildTask->run(future);
     }
 
     void updateProjectWhenDone(ProjectExplorer::Project *project)
@@ -103,11 +92,29 @@ public:
         return m_failed;
     }
 
-public slots:
-    void finish(QString qmldumpPath)
+private slots:
+    void finish(int qtId, const QString &output)
     {
-        deleteLater();
-        qmlDumpBuilds()->remove(m_version.uniqueId());
+        QtVersion *version = QtVersionManager::instance()->version(qtId);
+
+        QString errorMessage;
+        if (!version) {
+            m_failed = true;
+            errorMessage = QString::fromLatin1("Qt version became invalid");
+        } else {
+            version->invalidateCache();
+
+            if (version->qmlDumpTool().isEmpty()) {
+                m_failed = true;
+                errorMessage = QString::fromLatin1("Could not build QML plugin dumping helper for %1\n"
+                                                   "Output:\n%2").
+                        arg(version->displayName(), output);
+            }
+        }
+
+        if (m_failed) {
+            qWarning("%s", qPrintable(errorMessage));
+        }
 
         // update qmldump path for all the project
         QmlJS::ModelManagerInterface *modelManager = QmlJS::ModelManagerInterface::instance();
@@ -116,14 +123,19 @@ public slots:
 
         foreach (ProjectExplorer::Project *project, m_projectsToUpdate) {
             QmlJS::ModelManagerInterface::ProjectInfo projectInfo = modelManager->projectInfo(project);
-            projectInfo.qmlDumpPath = qmldumpPath;
-            projectInfo.qmlDumpEnvironment = m_version.qmlToolsEnvironment();
+            projectInfo.qmlDumpPath = version->qmlDumpTool();
+            projectInfo.qmlDumpEnvironment = version->qmlToolsEnvironment();
             modelManager->updateProjectInfo(projectInfo);
         }
+
+        // clean up
+        qmlDumpBuilds()->remove(qtId);
+        deleteLater();
     }
 
 private:
     QSet<ProjectExplorer::Project *> m_projectsToUpdate;
+    Internal::DebuggingHelperBuildTask *m_buildTask; // deletes itself after run()
     QtVersion m_version;
     bool m_failed;
 };

src/plugins/qt4projectmanager/qtoptionspage.cpp

@@ -65,7 +65,6 @@ enum ModelRoles { BuildLogRole = Qt::UserRole, BuildRunningRole = Qt::UserRole +
 using namespace Qt4ProjectManager;
 using namespace Qt4ProjectManager::Internal;
 
-
 ///
 // QtOptionsPage
 ///
@@ -288,21 +287,24 @@ QtVersion *QtOptionsPageWidget::currentVersion() const
     return 0;
 }
 
-static inline int findVersionByName(const QList<QSharedPointerQtVersion> &l, const QString &name)
+static inline int findVersionById(const QList<QSharedPointerQtVersion> &l, int id)
 {
     const int size = l.size();
     for (int i = 0; i < size; i++)
-        if (l.at(i)->displayName() == name)
+        if (l.at(i)->uniqueId() == id)
             return i;
     return -1;
 }
 
 // Update with results of terminated helper build
-void QtOptionsPageWidget::debuggingHelperBuildFinished(const QString &name, const QString &output)
+void QtOptionsPageWidget::debuggingHelperBuildFinished(int qtVersionId, const QString &output)
 {
-    const int index = findVersionByName(m_versions, name);
+    const int index = findVersionById(m_versions, qtVersionId);
     if (index == -1)
         return; // Oops, somebody managed to delete the version
+
+    m_versions.at(index)->invalidateCache();
+
     // Update item view
     QTreeWidgetItem *item = treeItemForIndex(index);
     QTC_ASSERT(item, return)
@@ -338,8 +340,9 @@ void QtOptionsPageWidget::buildDebuggingHelper()
     item->setData(2, BuildRunningRole, QVariant(true));
 
     // Run a debugging helper build task in the background.
-    DebuggingHelperBuildTask *buildTask = new DebuggingHelperBuildTask(m_versions.at(index));
-    connect(buildTask, SIGNAL(finished(QString,QString)), this, SLOT(debuggingHelperBuildFinished(QString,QString)),
+    DebuggingHelperBuildTask *buildTask = new DebuggingHelperBuildTask(m_versions.at(index).data(),
+                                                                       DebuggingHelperBuildTask::AllTools);
+    connect(buildTask, SIGNAL(finished(int,QString)), this, SLOT(debuggingHelperBuildFinished(int,QString)),
             Qt::QueuedConnection);
     QFuture<void> task = QtConcurrent::run(&DebuggingHelperBuildTask::run, buildTask);
     const QString taskName = tr("Building helpers");

src/plugins/qt4projectmanager/qtoptionspage.h

@@ -34,6 +34,7 @@
 #define QTOPTIONSPAGE_H
 
 #include <coreplugin/dialogs/ioptionspage.h>
+#include <utils/environment.h>
 
 #include <QtCore/QSharedPointer>
 #include <QtCore/QFutureInterface>
@@ -112,7 +113,7 @@ private slots:
     void msvcVersionChanged();
     void buildDebuggingHelper();
     void slotShowDebuggingBuildLog();
-    void debuggingHelperBuildFinished(const QString &versionName, const QString &output);
+    void debuggingHelperBuildFinished(int qtVersionId, const QString &output);
 
 private:
     void showDebuggingBuildLog(const QTreeWidgetItem *currentItem);

src/plugins/qt4projectmanager/qtversionmanager.cpp

@@ -1834,73 +1834,7 @@ bool QtVersion::isQt64Bit() const
 #endif
 }
 
-bool QtVersion::buildDebuggingHelperLibrary(QFutureInterface<void> &future,
-                                            bool onlyQmlDump,
-                                            QString *output, QString *errorMessage)
-{
-    const QString qtInstallData = versionInfo().value("QT_INSTALL_DATA");
-    if (qtInstallData.isEmpty()) {
-        *errorMessage =
-                QCoreApplication::translate("QtVersion",
-                                            "Cannot determine the installation path for Qt version '%1'.").
-                                             arg(displayName());
-        return false;
-    }
-    Utils::Environment env = Utils::Environment::systemEnvironment();
-    addToEnvironment(env);
-
-    // TODO: the debugging helper doesn't comply to actual tool chain yet
-    QList<QSharedPointer<ProjectExplorer::ToolChain> > alltc = toolChains();
-    ProjectExplorer::ToolChain *tc = alltc.isEmpty() ? 0 : alltc.first().data();
-    if (!tc) {
-        *errorMessage = QCoreApplication::translate("QtVersion", "The Qt Version has no toolchain.");
-        return false;
-    }
-    tc->addToEnvironment(env);
-    const QString target = (tc->type() == ProjectExplorer::ToolChain_GCC_MAEMO ? QLatin1String("-unix") : QLatin1String(""));
-
-    // invalidate cache
-    m_versionInfoUpToDate = false;
-
-    if (!onlyQmlDump) {
-        const QString gdbHelperDirectory = DebuggingHelperLibrary::copy(qtInstallData, errorMessage);
-        if (gdbHelperDirectory.isEmpty())
-            return false;
-        if (!DebuggingHelperLibrary::build(gdbHelperDirectory, tc->makeCommand(),
-                                           qmakeCommand(), mkspec(), env,
-                                           target, output, errorMessage))
-            return false;
-
-        future.setProgressValue(2);
-
-        if (QmlObserverTool::canBuild(this)) {
-            const QString toolDirectory = QmlObserverTool::copy(qtInstallData, errorMessage);
-            if (toolDirectory.isEmpty())
-                return false;
-            if (!QmlObserverTool::build(toolDirectory, tc->makeCommand(),
-                                        qmakeCommand(), mkspec(), env, target, output, errorMessage))
-                return false;
-        } else {
-            output->append(QCoreApplication::translate("Qt4ProjectManager::QtVersion", "Warning: Cannot build QMLObserver; Qt version must be 4.7.1 or higher."));
-        }
-        future.setProgressValue(3);
-    }
-
-    if (QmlDumpTool::canBuild(this)) {
-        const QString qmlDumpToolDirectory = QmlDumpTool::copy(qtInstallData, errorMessage);
-        if (qmlDumpToolDirectory.isEmpty())
-            return false;
-        if (!QmlDumpTool::build(qmlDumpToolDirectory, tc->makeCommand(),
-                                qmakeCommand(), mkspec(), env, target, output, errorMessage))
-            return false;
-
-    } else {
-//        output->append(QCoreApplication::translate("Qt4ProjectManager::QtVersion", "Warning: Cannot build qmldump; Qt version must be 4.7.1 or higher."));
-    }
-    future.setProgressValue(4);
-
-    // invalidate cache once more
+void QtVersion::invalidateCache()
+{
     m_versionInfoUpToDate = false;
-
-    return true;
 }

src/plugins/qt4projectmanager/qtversionmanager.h

@@ -138,10 +138,7 @@ public:
     bool hasQmlObserver() const;
     Utils::Environment qmlToolsEnvironment() const;
 
-    // Builds a debugging library
-    // returns the output of the commands
-    bool buildDebuggingHelperLibrary(QFutureInterface<void> &future, bool onlyQmlDump,
-                                     QString *output, QString *errorMessage);
+    void invalidateCache();
 
     bool hasExamples() const;
     QString examplesPath() const;