diff --git a/qrenderdoc/Code/Interface/PersistantConfig.h b/qrenderdoc/Code/Interface/PersistantConfig.h index e3d5d1269..63c973129 100644 --- a/qrenderdoc/Code/Interface/PersistantConfig.h +++ b/qrenderdoc/Code/Interface/PersistantConfig.h @@ -606,6 +606,9 @@ public: // Runtime list of dynamically allocated hosts. // Saved to/from private RemoteHostList in CONFIG_SETTINGS() // This is documented above in the docstring, similar to the values in CONFIG_SETTINGS() + // This must only be accessed on the UI thread to prevent races. For access on other threads (e.g. + // a background/asynchronous update), take a copy on the UI thread, update it in the background, + // then apply the updates. DOCUMENT(""); rdcarray RemoteHosts; #endif diff --git a/qrenderdoc/Code/Interface/RemoteHost.h b/qrenderdoc/Code/Interface/RemoteHost.h index 59b387315..1ba9ff4e7 100644 --- a/qrenderdoc/Code/Interface/RemoteHost.h +++ b/qrenderdoc/Code/Interface/RemoteHost.h @@ -37,6 +37,14 @@ public: VARIANT_CAST(RemoteHost); + DOCUMENT(""); + bool operator==(const RemoteHost &o) const + { + return hostname == o.hostname && friendlyName == o.friendlyName && runCommand == o.runCommand && + serverRunning == o.serverRunning && connected == o.connected && busy == o.busy && + versionMismatch == o.versionMismatch; + } + bool operator!=(const RemoteHost &o) const { return !(*this == o); } DOCUMENT( "Ping the host to check current status - if the server is running, connection status, etc."); void CheckStatus(); diff --git a/qrenderdoc/Windows/MainWindow.cpp b/qrenderdoc/Windows/MainWindow.cpp index 5325cb1a6..486f864b5 100644 --- a/qrenderdoc/Windows/MainWindow.cpp +++ b/qrenderdoc/Windows/MainWindow.cpp @@ -161,6 +161,9 @@ MainWindow::MainWindow(ICaptureContext &ctx) : QMainWindow(NULL), ui(new Ui::Mai m_RemoteProbe = new LambdaThread([this]() { while(m_RemoteProbeSemaphore.available()) { + // do a remoteProbe immediately to populate the android hosts list on startup. + remoteProbe(); + // do several small sleeps so we can respond quicker when we need to shut down for(int i = 0; i < 50; i++) { @@ -168,7 +171,6 @@ MainWindow::MainWindow(ICaptureContext &ctx) : QMainWindow(NULL), ui(new Ui::Mai if(!m_RemoteProbeSemaphore.available()) return; } - remoteProbe(); } }); m_RemoteProbe->start(); @@ -293,14 +295,6 @@ MainWindow::MainWindow(ICaptureContext &ctx) : QMainWindow(NULL), ui(new Ui::Mai ui->action_Recompress_Capture->setEnabled(false); - LambdaThread *th = new LambdaThread([this]() { - m_Ctx.Config().AddAndroidHosts(); - for(RemoteHost *host : m_Ctx.Config().RemoteHosts) - host->CheckStatus(); - }); - th->selfDelete(true); - th->start(); - #if defined(Q_OS_WIN32) if(GetModuleHandleA("rdocself.dll")) { @@ -1513,20 +1507,61 @@ void MainWindow::remoteProbe() { if(!m_Ctx.IsCaptureLoaded() && !m_Ctx.IsCaptureLoading()) { - GUIInvoke::call(this, [this] { m_Ctx.Config().AddAndroidHosts(); }); + GUIInvoke::call(this, [this] { + m_Ctx.Config().AddAndroidHosts(); - for(RemoteHost *host : m_Ctx.Config().RemoteHosts) + // update the latest list by copy. Note this lock only protects m_ProbeRemoteHosts, not the + // actual RemoteHosts list itself - that is only accessed on the UI thread so is not locked. + { + QMutexLocker lock(&m_ProbeRemoteHostsLock); + m_ProbeRemoteHosts.clear(); + for(RemoteHost *host : m_Ctx.Config().RemoteHosts) + m_ProbeRemoteHosts.push_back(*host); + } + }); + + // fetch the latest list + rdcarray hosts; + { + QMutexLocker lock(&m_ProbeRemoteHostsLock); + hosts = m_ProbeRemoteHosts; + } + + QList> updates; + + for(RemoteHost &host : hosts) { // don't mess with a host we're connected to - this is handled anyway - if(host->connected) + if(host.connected) continue; - host->CheckStatus(); + RemoteHost updated = host; + + updated.CheckStatus(); // bail as soon as we notice that we're done if(!m_RemoteProbeSemaphore.available()) return; + + if(updated != host) + updates.push_back(qMakePair(host, updated)); } + + GUIInvoke::call(this, [this, updates] { + // this is a bit naive - it suffers from the A-B-A problem where the host could be updated + // then restored to its original state before we notice. In this case though we don't care + // about that, we just don't want to trash some important changes like the connected status + // changing or something. + + for(const QPair &u : updates) + { + for(RemoteHost *host : m_Ctx.Config().RemoteHosts) + { + if(*host == u.first) + *host = u.second; + } + } + }); } } diff --git a/qrenderdoc/Windows/MainWindow.h b/qrenderdoc/Windows/MainWindow.h index bfb8d2360..50afa54e0 100644 --- a/qrenderdoc/Windows/MainWindow.h +++ b/qrenderdoc/Windows/MainWindow.h @@ -26,6 +26,7 @@ #include #include +#include #include #include #include "3rdparty/toolwindowmanager/ToolWindowManager.h" @@ -201,6 +202,11 @@ private: QSemaphore m_RemoteProbeSemaphore; LambdaThread *m_RemoteProbe; + // m_ProbeRemoteHosts is covered by a lock. On the UI thread we copy it from the config regularly, + // then apply any updates back. + rdcarray m_ProbeRemoteHosts; + QMutex m_ProbeRemoteHostsLock; + QNetworkAccessManager *m_NetManager; bool m_messageAlternate = false;