Fix download manager crash and dropped downloads

src/downloadlist.cpp

@@ -114,8 +114,11 @@ QVariant DownloadList::data(const QModelIndex& index, int role) const
   bool pendingDownload = index.row() >= m_manager.numTotalDownloads();
   if (role == Qt::DisplayRole) {
     if (pendingDownload) {
-      const DownloadManager::PendingDownload pending =
-          m_manager.getPendingDownload(index.row() - m_manager.numTotalDownloads());
+      DownloadManager::PendingDownload pending;
+      if (!m_manager.tryGetPendingDownload(index.row() - m_manager.numTotalDownloads(),
+                                           pending)) {
+        return QVariant();
+      }
       switch (index.column()) {
       case COL_NAME:
         return tr("< game %1 mod %2 file %3 >")

src/downloadmanager.cpp

@@ -280,8 +280,13 @@ DownloadManager::DownloadManager(NexusInterface* nexusInterface, QObject* parent
       m_ParentWidget(nullptr)
 {
   m_OrganizerCore = dynamic_cast<OrganizerCore*>(parent);
-  connect(&m_DirWatcher, &DirWatcherManager::directoryChanged, this,
-          &DownloadManager::refreshList);
+
+  m_RefreshTimer.setSingleShot(true);
+  m_RefreshTimer.setInterval(1000);
+  connect(&m_RefreshTimer, &QTimer::timeout, this, &DownloadManager::refreshList);
+  connect(&m_DirWatcher, &DirWatcherManager::directoryChanged, this, [this]() {
+    m_RefreshTimer.start();
+  });
 }
 
 DownloadManager::~DownloadManager()
@@ -1366,6 +1371,15 @@ DownloadManager::PendingDownload DownloadManager::getPendingDownload(int index)
   return m_PendingDownloads.at(index);
 }
 
+bool DownloadManager::tryGetPendingDownload(int index, PendingDownload& download) const
+{
+  if ((index < 0) || (index >= m_PendingDownloads.size())) {
+    return false;
+  }
+  download = m_PendingDownloads.at(index);
+  return true;
+}
+
 QString DownloadManager::getFilePath(int index) const
 {
   if ((index < 0) || (index >= m_ActiveDownloads.size())) {

src/downloadmanager.h

@@ -32,6 +32,7 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 #include <QSettings>
 #include <QStringList>
 #include <QTime>
+#include <QTimer>
 #include <QUrl>
 #include <QVector>
 #include <boost/accumulators/accumulators.hpp>
@@ -384,6 +385,12 @@ class DownloadManager : public QObject
    */
   PendingDownload getPendingDownload(int index);
 
+  /**
+   * @brief retrieve the info of a pending download without throwing
+   * @return true if the index was valid and download was populated
+   */
+  bool tryGetPendingDownload(int index, PendingDownload& download) const;
+
   /**
    * @brief Resolve a view row to a stable DownloadID.
    *
@@ -809,6 +816,8 @@ private slots:
 
   DirWatcherManager m_DirWatcher;
 
+  QTimer m_RefreshTimer;
+
   // nesting depth of active ModelResetGuard scopes; see its docs
   int m_modelResetDepth = 0;
 

src/multiprocess.cpp

@@ -1,16 +1,60 @@
 #include "multiprocess.h"
 #include "utility.h"
 #include <QLocalSocket>
+#include <QThread>
 #include <log.h>
 #include <report.h>
 
-static const char s_Key[]  = "mo-43d1a3ad-eeb0-4818-97c9-eda5216c29b5";
-static const int s_Timeout = 5000;
+static const char s_Key[]         = "mo-43d1a3ad-eeb0-4818-97c9-eda5216c29b5";
+static const int s_Timeout        = 5000;
+static const int s_DeliverTimeout = 30000;
 
 using MOBase::reportError;
 
+MessageReceiver::MessageReceiver(QString key) : m_Key(std::move(key)), m_Server(nullptr)
+{}
+
+void MessageReceiver::start()
+{
+  m_Server = new QLocalServer(this);
+  connect(m_Server, &QLocalServer::newConnection, this,
+          &MessageReceiver::onNewConnection);
+  // has to be called before listen
+  m_Server->setSocketOptions(QLocalServer::WorldAccessOption);
+
+  if (!m_Server->listen(s_Key)) {
+    MOBase::log::error("failed to listen for secondary processes: {}",
+                       m_Server->errorString().toStdString());
+  }
+}
+
+void MessageReceiver::onNewConnection()
+{
+  while (QLocalSocket* socket = m_Server->nextPendingConnection()) {
+    connect(socket, &QLocalSocket::readyRead, this, [this, socket]() {
+      readFrom(socket);
+    });
+    connect(socket, &QLocalSocket::disconnected, socket, &QObject::deleteLater);
+
+    readFrom(socket);
+  }
+}
+
+void MessageReceiver::readFrom(QLocalSocket* socket)
+{
+  const QByteArray data = socket->readAll();
+  if (data.isEmpty()) {
+    return;
+  }
+
+  emit messageReceived(QString::fromUtf8(data));
+
+  socket->disconnectFromServer();
+}
+
 MOMultiProcess::MOMultiProcess(bool allowMultiple, QObject* parent)
-    : QObject(parent), m_Ephemeral(false), m_OwnsSM(false)
+    : QObject(parent), m_Ephemeral(false), m_OwnsSM(false), m_ServerThread(nullptr),
+      m_Receiver(nullptr)
 {
   m_SharedMem.setKey(s_Key);
 
@@ -31,11 +75,27 @@ MOMultiProcess::MOMultiProcess(bool allowMultiple, QObject* parent)
   }
 
   if (m_OwnsSM) {
-    connect(&m_Server, SIGNAL(newConnection()), this, SLOT(receiveMessage()),
-            Qt::QueuedConnection);
-    // has to be called before listen
-    m_Server.setSocketOptions(QLocalServer::WorldAccessOption);
-    m_Server.listen(s_Key);
+    m_ServerThread = new QThread(this);
+    m_ServerThread->setObjectName("mo-ipc-server");
+
+    m_Receiver = new MessageReceiver(s_Key);
+    m_Receiver->moveToThread(m_ServerThread);
+
+    connect(m_ServerThread, &QThread::started, m_Receiver, &MessageReceiver::start);
+    connect(m_ServerThread, &QThread::finished, m_Receiver, &QObject::deleteLater);
+
+    connect(m_Receiver, &MessageReceiver::messageReceived, this,
+            &MOMultiProcess::messageSent, Qt::QueuedConnection);
+
+    m_ServerThread->start();
+  }
+}
+
+MOMultiProcess::~MOMultiProcess()
+{
+  if (m_ServerThread) {
+    m_ServerThread->quit();
+    m_ServerThread->wait();
   }
 }
 
@@ -54,7 +114,7 @@ void MOMultiProcess::sendMessage(const QString& message)
     }
 
     // other process may be just starting up
-    socket.connectToServer(s_Key, QIODevice::WriteOnly);
+    socket.connectToServer(s_Key, QIODevice::ReadWrite);
     connected = socket.waitForConnected(s_Timeout);
   }
 
@@ -72,34 +132,5 @@ void MOMultiProcess::sendMessage(const QString& message)
     }
   }
 
-  socket.disconnectFromServer();
-  socket.waitForDisconnected();
-}
-
-void MOMultiProcess::receiveMessage()
-{
-  QLocalSocket* socket = m_Server.nextPendingConnection();
-  if (!socket) {
-    return;
-  }
-
-  if (!socket->waitForReadyRead(s_Timeout)) {
-    // check if there are bytes available; if so, it probably means the data was
-    // already received by the time waitForReadyRead() was called and the
-    // connection has been closed
-    const auto av = socket->bytesAvailable();
-
-    if (av <= 0) {
-      MOBase::log::error("failed to receive data from secondary process: {}",
-                         socket->errorString());
-
-      reportError(tr("failed to receive data from secondary process: %1")
-                      .arg(socket->errorString()));
-      return;
-    }
-  }
-
-  QString message = QString::fromUtf8(socket->readAll().constData());
-  emit messageSent(message);
-  socket->disconnectFromServer();
+  socket.waitForDisconnected(s_DeliverTimeout);
 }

src/multiprocess.h

@@ -4,6 +4,33 @@
 #include <QLocalServer>
 #include <QObject>
 #include <QSharedMemory>
+#include <QString>
+
+class QThread;
+class QLocalSocket;
+
+class MessageReceiver : public QObject
+{
+  Q_OBJECT
+
+public:
+  explicit MessageReceiver(QString key);
+
+public slots:
+  void start();
+
+signals:
+  void messageReceived(const QString& message);
+
+private slots:
+  void onNewConnection();
+
+private:
+  void readFrom(QLocalSocket* socket);
+
+  QString m_Key;
+  QLocalServer* m_Server;
+};
 
 /**
  * used to ensure only a single process of Mod Organizer is started and to
@@ -19,6 +46,8 @@ class MOMultiProcess : public QObject
   // disconnected from the shared memory
   explicit MOMultiProcess(bool allowMultiple, QObject* parent = 0);
 
+  ~MOMultiProcess();
+
   /**
    * @return true if this process's job is to forward data to the primary
    *              process through shared memory
@@ -46,17 +75,13 @@ class MOMultiProcess : public QObject
    **/
   void messageSent(const QString& message);
 
-public slots:
-
-private slots:
-
-  void receiveMessage();
-
 private:
   bool m_Ephemeral;
   bool m_OwnsSM;
   QSharedMemory m_SharedMem;
-  QLocalServer m_Server;
+
+  QThread* m_ServerThread;
+  MessageReceiver* m_Receiver;
 };
 
 #endif  // MODORGANIZER_MOMULTIPROCESS_INCLUDED

src/version.rc

@@ -3,14 +3,14 @@
 // If VS_FF_PRERELEASE is not set, MO labels the build as a release and uses VER_FILEVERSION to determine version number.
 // Otherwise, if letters are used in VER_FILEVERSION_STR, uses the full MOBase::VersionInfo parser
 // Otherwise, uses the numbers from VER_FILEVERSION and sets the release type as pre-alpha
-#define VER_FILEVERSION     2,5,2
-#define VER_FILEVERSION_STR "2.5.2\0"
+#define VER_FILEVERSION     2,5,3
+#define VER_FILEVERSION_STR "2.5.3beta12\0"
 
 VS_VERSION_INFO VERSIONINFO
 FILEVERSION     VER_FILEVERSION
 PRODUCTVERSION  VER_FILEVERSION
 FILEFLAGSMASK   VS_FFI_FILEFLAGSMASK
-FILEFLAGS       (0)
+FILEFLAGS       VS_FF_PRERELEASE
 FILEOS          VOS__WINDOWS32
 FILETYPE        VFT_APP
 FILESUBTYPE     (0)