Debugger [CDB]: Release COM interfaces on RunControl finished.

Ensure that at most one instance of the CDBEngine is alive.
Prevent inconsistent debugger states if several sessions are started.
Squashed cherry-picked commits from master correcting the
plugin state handling (2efdeb5cd0,
57f7616286).

Reviewed-by: hjk
Task-number: QTCREATORBUG-2894

src/plugins/debugger/cdb/cdbengine.cpp

@@ -168,6 +168,10 @@ bool CdbEnginePrivate::init(QString *errorMessage)
 DebuggerEngine *CdbEngine::create(const DebuggerStartParameters &sp,
                                        QString *errorMessage)
 {
+    if (!CdbCore::CoreEngine::interfacesAvailable()) {
+        *errorMessage = CdbEngine::tr("An instance of the CDB engine is still running; cannot create an a new instance.");
+        return 0;
+    }
     CdbEngine *rc = new CdbEngine(sp);
     if (rc->m_d->init(errorMessage)) {
         rc->syncDebuggerPaths();
@@ -563,6 +567,7 @@ void CdbEngine::processTerminated(unsigned long exitCode)
 
 bool CdbEnginePrivate::endInferior(bool detachOnly, QString *errorMessage)
 {
+    QTC_ASSERT(hasInterfaces(), return true; )
     // Prevent repeated invocation.
     const bool hasHandles = m_hDebuggeeProcess != NULL;
     if (debugCDBExecution)
@@ -635,6 +640,8 @@ void CdbEnginePrivate::endDebugging(bool detachOnly)
     if (debugCDBExecution)
         qDebug("endDebugging() detach=%d, state=%s", detachOnly, DebuggerEngine::stateName(m_engine->state()));
 
+    QTC_ASSERT(hasInterfaces(), return; )
+
     switch (m_engine->state()) {
     case DebuggerNotReady:
     case EngineShutdownOk:
@@ -659,6 +666,8 @@ void CdbEnginePrivate::endDebugging(bool detachOnly)
         m_engine->showMessage(errorMessage, LogError);
         m_engine->notifyEngineShutdownFailed();
     }
+    // At this point release interfaces as we might be kept around by the run control.
+    releaseInterfaces();
 }
 
 void CdbEngine::detachDebugger()

src/plugins/debugger/cdb/coreengine.cpp

@@ -49,6 +49,8 @@ static const char *debugCreateFuncC = "DebugCreate";
 
 enum { debug = 0 };
 
+Q_GLOBAL_STATIC(QString, baseImagePath)
+
 static inline QString msgLibLoadFailed(const QString &lib, const QString &why)
 {
     return QCoreApplication::translate("Debugger::Cdb",
@@ -223,6 +225,8 @@ bool DebuggerEngineLibrary::init(const QString &path,
     return true;
 }
 
+CoreEngine *CoreEngine::m_instance = 0;
+
 // ------ Engine
 CoreEngine::CoreEngine(QObject *parent) :
     QObject(parent),
@@ -231,32 +235,70 @@ CoreEngine::CoreEngine(QObject *parent) :
     m_lastTimerModuleCount(0),
     m_modulesLoadedEmitted(true)
 {
+    m_instance = this;
 }
 
 CoreEngine::~CoreEngine()
 {
+    releaseInterfaces();
+    m_instance = 0;
+}
 
+bool CoreEngine::hasInterfaces() const
+{
+    return m_cif.debugClient != 0;
+}
+
+bool CoreEngine::interfacesAvailable()
+{
+    return CoreEngine::m_instance == 0 ||
+            !CoreEngine::m_instance->hasInterfaces();
+}
+
+void CoreEngine::releaseInterfaces()
+{
     if (m_cif.debugClient) {
         m_cif.debugClient->SetOutputCallbacksWide(0);
         m_cif.debugClient->SetEventCallbacksWide(0);
         m_cif.debugClient->Release();
+        m_cif.debugClient = 0;
     }
-    if (m_cif.debugControl)
+    if (m_cif.debugControl) {
         m_cif.debugControl->Release();
-    if (m_cif.debugSystemObjects)
+        m_cif.debugControl = 0;
+    }
+    if (m_cif.debugSystemObjects) {
         m_cif.debugSystemObjects->Release();
-    if (m_cif.debugSymbols)
+        m_cif.debugSystemObjects =0;
+    }
+
+    if (m_cif.debugSymbols) {
         m_cif.debugSymbols->Release();
-    if (m_cif.debugRegisters)
+        m_cif.debugSymbols = 0;
+    }
+
+    if (m_cif.debugRegisters) {
         m_cif.debugRegisters->Release();
-    if (m_cif.debugDataSpaces)
+        m_cif.debugRegisters = 0;
+    }
+
+    if (m_cif.debugDataSpaces) {
         m_cif.debugDataSpaces->Release();
-    if (m_cif.debugAdvanced)
+        m_cif.debugDataSpaces = 0;
+    }
+
+    if (m_cif.debugAdvanced) {
         m_cif.debugAdvanced->Release();
+        m_cif.debugAdvanced = 0;
+    }
 }
 
 bool CoreEngine::init(const QString &dllEnginePath, QString *errorMessage)
 {
+    if (!CoreEngine::interfacesAvailable()) {
+        *errorMessage = QString::fromLatin1("Internal error: The COM interfaces are already in use.");
+        return false;
+    }
     enum {  bufLen = 10240 };
     // Load the DLL
     DebuggerEngineLibrary lib;
@@ -293,13 +335,18 @@ bool CoreEngine::init(const QString &dllEnginePath, QString *errorMessage)
         return false;
     }
 
-    WCHAR buf[bufLen];
-    hr = m_cif.debugSymbols->GetImagePathWide(buf, bufLen, 0);
-    if (FAILED(hr)) {
-        *errorMessage = msgComFailed("GetImagePathWide", hr);
-        return false;
+    // Query inherited image path from environment only once as it is remembered.
+    static bool firstInstance = true;
+    if (firstInstance) {
+        firstInstance = false;
+        WCHAR buf[bufLen];
+        hr = m_cif.debugSymbols->GetImagePathWide(buf, bufLen, 0);
+        if (FAILED(hr)) {
+            *errorMessage = msgComFailed("GetImagePathWide", hr);
+            return false;
+        }
+        *baseImagePath() = QString::fromWCharArray(buf);
     }
-    m_baseImagePath = QString::fromWCharArray(buf);
 
     hr = lib.debugCreate( __uuidof(IDebugRegisters2), reinterpret_cast<void**>(&m_cif.debugRegisters));
     if (FAILED(hr)) {
@@ -430,10 +477,11 @@ bool CoreEngine::startDebuggerWithExecutable(const QString &workingDirectory,
     // Set image path
     const QFileInfo fi(filename);
     QString imagePath = QDir::toNativeSeparators(fi.absolutePath());
-    if (!m_baseImagePath.isEmpty()) {
+    if (!baseImagePath()->isEmpty()) {
         imagePath += QLatin1Char(';');
-        imagePath += m_baseImagePath;
+        imagePath += baseImagePath();
     }
+
     HRESULT hr = m_cif.debugSymbols->SetImagePathWide(reinterpret_cast<PCWSTR>(imagePath.utf16()));
     if (FAILED(hr)) {
         *errorMessage = tr("Unable to set the image path to %1: %2").arg(imagePath, msgComFailed("SetImagePathWide", hr));

src/plugins/debugger/cdb/coreengine.h

@@ -69,6 +69,11 @@ public:
     explicit CoreEngine(QObject *parent = 0);
     virtual ~CoreEngine();
 
+    // Preliminary release interfaces.
+    void releaseInterfaces();
+    bool hasInterfaces() const;
+    static bool interfacesAvailable();
+
     bool init(const QString &dllEnginePath, QString *errorMessage);
     // code level/output
 
@@ -179,11 +184,11 @@ private:
     void setModuleCount(unsigned m);
     void resetModuleLoadTimer();
 
+    static CoreEngine *m_instance;
     ComInterfaces m_cif;
     DebugOutputBasePtr m_debugOutput;
     DebugEventCallbackBasePtr m_debugEventCallback;
     QString m_dbengDLL;
-    QString m_baseImagePath;
     int m_watchTimer;
     unsigned m_moduleCount;
     unsigned m_lastTimerModuleCount;

src/plugins/debugger/debuggerengine.cpp

@@ -1182,7 +1182,7 @@ void DebuggerEngine::notifyEngineSetupFailed()
     setState(EngineSetupFailed);
     QTC_ASSERT(d->m_runControl, return);
     d->m_runControl->startFailed();
-    d->queueShutdownEngine();
+    setState(DebuggerFinished);
 }
 
 void DebuggerEngine::notifyEngineSetupOk()
@@ -1244,7 +1244,7 @@ void DebuggerEngine::notifyEngineRunFailed()
     d->m_progress.reportCanceled();
     d->m_progress.reportFinished();
     setState(EngineRunFailed);
-    d->queueShutdownInferior();
+    d->queueShutdownEngine();
 }
 
 void DebuggerEngine::notifyEngineRunAndInferiorRunOk()

src/plugins/debugger/debuggerplugin.cpp

@@ -172,6 +172,13 @@
 // Transitions marked by '*' are done asynchronously.
 // The GdbEngine->setupEngine() function is described in more detail below.
 //
+// The engines are responsible for local roll-back to the last
+// acknowledged state before calling notify*Failed. I.e. before calling
+// notifyEngineSetupFailed() any process started during setupEngine()
+// so far must be terminated.
+//
+//
+//
 //                        DebuggerNotReady
 //                               +
 //                      EngineSetupRequested