WinDebugInterface: Remove blocking connection to main thread

The debugOutput() signal was connected via BlockingQueuedConnection and
blocks the WinDebugInterface thread until the signal is received. If we
signal "buffer ready" afterwards, all OutputDebugString calls (of all
processes of the system) that wait for "buffer ready" also wait for our
internal signal delivery. This can lead to hangs in circumstances where
the event loop is currently unable to deliver the signal.

Remove the blocking queued connection, and introduce an internal
_q_debugOutputReady() signal that is emitted whenever new debug output
is available. This signal is queued-connected to a dispatchDebugOutput
function, which is running in the main thread. That function retrieves
the data from the WinDebugInterfaceThread and emits debugOutput(qint64,
QString). This massively reduces the event queue load without blocking
the WinDebugInterface thread.

Task-number: QTCREATORBUG-20640
Change-Id: I91f8f794af8da2a695c2b897f678844b142a5991
Reviewed-by: David Schulz <david.schulz@qt.io>
Reviewed-by: Alessandro Portale <alessandro.portale@qt.io>
Reviewed-by: Eike Ziller <eike.ziller@qt.io>

src/plugins/projectexplorer/applicationlauncher.cpp

@@ -160,7 +160,7 @@ ApplicationLauncherPrivate::ApplicationLauncherPrivate(ApplicationLauncher *pare
     connect(WinDebugInterface::instance(), &WinDebugInterface::cannotRetrieveDebugOutput,
             this, &ApplicationLauncherPrivate::cannotRetrieveLocalDebugOutput);
     connect(WinDebugInterface::instance(), &WinDebugInterface::debugOutput,
-            this, &ApplicationLauncherPrivate::checkLocalDebugOutput, Qt::BlockingQueuedConnection);
+            this, &ApplicationLauncherPrivate::checkLocalDebugOutput);
 #endif
 }
 

src/plugins/projectexplorer/windebuginterface.cpp

@@ -27,10 +27,11 @@
 
 #ifdef Q_OS_WIN
 
-#include <windows.h>
+#include <utils/qtcassert.h>
 #include <QCoreApplication>
-#include <QMap>
-#include <QTime>
+#include <qt_windows.h>
+
+#include <algorithm>
 
 /*!
     \class ProjectExplorer::Internal::WinDebugInterface
@@ -67,6 +68,8 @@ WinDebugInterface::WinDebugInterface(QObject *parent) :
     m_instance = this;
     m_creatorPid = QCoreApplication::applicationPid();
     setObjectName(QLatin1String("WinDebugInterfaceThread"));
+    connect(this, &WinDebugInterface::_q_debugOutputReady,
+            this, &WinDebugInterface::dispatchDebugOutput, Qt::QueuedConnection);
 }
 
 WinDebugInterface::~WinDebugInterface()
@@ -131,41 +134,18 @@ bool WinDebugInterface::runLoop()
 
     SetEvent(m_bufferReadyEvent);
 
-    QTime timer; // time since last signal sent
-    timer.start();
-
-    QMap<qint64, QString> delayedMessages;
-
-    auto flushMessages = [this, &delayedMessages, &timer](){
-        auto it = delayedMessages.constBegin();
-        auto end = delayedMessages.constEnd();
-        for (; it != end; ++it)
-            emit debugOutput(it.key(), it.value());
-        delayedMessages.clear();
-        timer.start();
-    };
-
     while (true) {
-        DWORD timeout = INFINITE;
-        if (!delayedMessages.isEmpty()) // if we have delayed message, don't wait forever
-            timeout = qMax(60 - timer.elapsed(), 1);
-        const DWORD ret = WaitForMultipleObjects(HandleCount, m_waitHandles, FALSE, timeout);
-
+        const DWORD ret = WaitForMultipleObjects(HandleCount, m_waitHandles, FALSE, INFINITE);
         if (ret == WAIT_FAILED || ret - WAIT_OBJECT_0 == TerminateEventHandle) {
-            flushMessages();
+            std::lock_guard<std::mutex> guard(m_outputMutex);
+            emitReadySignal();
             break;
         }
-        if (ret == WAIT_TIMEOUT) {
-            flushMessages();
-            SetEvent(m_bufferReadyEvent);
-        } else if (ret - WAIT_OBJECT_0 == DataReadyEventHandle) {
+        if (ret - WAIT_OBJECT_0 == DataReadyEventHandle) {
             if (*processId != m_creatorPid) {
-                if (timer.elapsed() < 60) {
-                    delayedMessages[*processId].append(QString::fromLocal8Bit(message));
-                } else {
-                    delayedMessages[*processId] += QString::fromLocal8Bit(message);
-                    flushMessages();
-                }
+                std::lock_guard<std::mutex> guard(m_outputMutex);
+                m_debugOutput[*processId].push_back(QString::fromLocal8Bit(message));
+                emitReadySignal();
             }
             SetEvent(m_bufferReadyEvent);
         }
@@ -173,6 +153,49 @@ bool WinDebugInterface::runLoop()
     return true;
 }
 
+void WinDebugInterface::emitReadySignal()
+{
+    // This function must be called from the WinDebugInterface thread only.
+    QTC_ASSERT(QThread::currentThread() == this, return);
+
+    if (m_debugOutput.empty() || m_readySignalEmitted)
+        return;
+
+    m_readySignalEmitted = true;
+    emit _q_debugOutputReady();
+}
+
+void WinDebugInterface::dispatchDebugOutput()
+{
+    // Called in the thread this object was created in, not in the WinDebugInterfaceThread.
+    QTC_ASSERT(QThread::currentThread() == QCoreApplication::instance()->thread(), return);
+
+    static size_t maxMessagesToSend = 100;
+    std::vector<std::pair<qint64, QString>> output;
+    bool hasMoreOutput = false;
+
+    m_outputMutex.lock();
+    for (auto &entry : m_debugOutput) {
+        std::vector<QString> &src = entry.second;
+        QString dst;
+        size_t n = std::min(maxMessagesToSend, src.size());
+        for (size_t i = 0; i < n; ++i)
+            dst += src.at(i);
+        src.erase(src.begin(), std::next(src.begin(), n));
+        if (!src.empty())
+            hasMoreOutput = true;
+        output.emplace_back(entry.first, std::move(dst));
+    }
+    if (!hasMoreOutput)
+        m_readySignalEmitted = false;
+    m_outputMutex.unlock();
+
+    for (auto p : output)
+        emit debugOutput(p.first, p.second);
+    if (hasMoreOutput)
+        emit _q_debugOutputReady();
+}
+
 } // namespace Internal
 } // namespace ProjectExplorer
 
@@ -193,6 +216,10 @@ void WinDebugInterface::run() { }
 
 bool WinDebugInterface::runLoop() { return false; }
 
+void WinDebugInterface::emitReadySignal() { }
+
+void WinDebugInterface::dispatchDebugOutput() { }
+
 } // namespace Internal
 } // namespace ProjectExplorer
 

src/plugins/projectexplorer/windebuginterface.h

@@ -25,8 +25,13 @@
 
 #pragma once
 
+#include <QMap>
 #include <QThread>
 
+#include <map>
+#include <mutex>
+#include <vector>
+
 namespace ProjectExplorer {
 namespace Internal {
 
@@ -45,12 +50,15 @@ public:
 signals:
     void debugOutput(qint64 pid, const QString &message);
     void cannotRetrieveDebugOutput();
+    void _q_debugOutputReady();
 
 private:
     enum Handles { DataReadyEventHandle, TerminateEventHandle, HandleCount };
 
     void run() override;
     bool runLoop();
+    void emitReadySignal();
+    void dispatchDebugOutput();
 
     static WinDebugInterface *m_instance;
 
@@ -59,6 +67,9 @@ private:
     Qt::HANDLE m_bufferReadyEvent = nullptr;
     Qt::HANDLE m_sharedFile = nullptr;
     void *m_sharedMem = nullptr;
+    std::mutex m_outputMutex;
+    bool m_readySignalEmitted = false;
+    std::map<qint64, std::vector<QString>> m_debugOutput;
 };
 
 } // namespace Internal