From ea829f96cad1a3ee794cffd024e6eec3bca81e59 Mon Sep 17 00:00:00 2001
From: Christian Kamm <mail@ckamm.de>
Date: Mon, 12 Aug 2019 08:23:40 +0200
Subject: [PATCH] Download: Don't trigger too many concurrent hash computations

Previously the job would only become "active" when the downloads
started. That meant that arbitrarily many hash computations could be
queued at the same time.

Since the the file was opened during future creation this could lead to
a "too many open files" problem if there were lots of new-new conflicts.

To change this:
- Make PropagateDownload become active when computing a hash
  asynchronously.
- Make the computation future open the file only once it gets run. This
  will make it less likely for this problem to occur even if thousands
  of these futures are queued.

For #7372
---
 src/common/checksums.cpp          | 17 +++++++----------
 src/libsync/propagatedownload.cpp |  2 ++
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/common/checksums.cpp b/src/common/checksums.cpp
index 28483046d..c0503cb72 100644
--- a/src/common/checksums.cpp
+++ b/src/common/checksums.cpp
@@ -225,14 +225,6 @@ QByteArray ComputeChecksum::checksumType() const
 void ComputeChecksum::start(const QString &filePath)
 {
     qCInfo(lcChecksums) << "Computing" << checksumType() << "checksum of" << filePath << "in a thread";
-    // make_unique() would be more appropriate, but QtConcurrent requires
-    // copyable types.
-    auto file = std::make_shared<QFile>(filePath);
-    if (!file->open(QIODevice::ReadOnly)) {
-        qCWarning(lcChecksums) << "Could not open file" << filePath << "for reading to compute a checksum" << file->errorString();
-        emit done(QByteArray(), QByteArray());
-        return;
-    }
 
     connect(&_watcher, &QFutureWatcherBase::finished,
         this, &ComputeChecksum::slotCalculationDone,
@@ -241,8 +233,13 @@ void ComputeChecksum::start(const QString &filePath)
     // Capturing "file" extends its lifetime to the lifetime of the new thread.
     // Bug: The thread will keep running even if ComputeChecksum is deleted.
     auto type = checksumType();
-    _watcher.setFuture(QtConcurrent::run([file, type]() {
-        return ComputeChecksum::computeNow(file.get(), type);
+    _watcher.setFuture(QtConcurrent::run([filePath, type]() {
+        QFile file(filePath);
+        if (!file.open(QIODevice::ReadOnly)) {
+            qCWarning(lcChecksums) << "Could not open file" << filePath << "for reading to compute a checksum" << file.errorString();
+            return QByteArray();
+        }
+        return ComputeChecksum::computeNow(&file, type);
     }));
 }
 
diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index 3cd647d54..120170382 100644
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -464,6 +464,7 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked()
         computeChecksum->setChecksumType(parseChecksumHeaderType(_item->_checksumHeader));
         connect(computeChecksum, &ComputeChecksum::done,
             this, &PropagateDownloadFile::conflictChecksumComputed);
+        propagator()->_activeJobList.append(this);
         computeChecksum->start(propagator()->getFilePath(_item->_file));
         return;
     }
@@ -473,6 +474,7 @@ void PropagateDownloadFile::startAfterIsEncryptedIsChecked()
 
 void PropagateDownloadFile::conflictChecksumComputed(const QByteArray &checksumType, const QByteArray &checksum)
 {
+    propagator()->_activeJobList.removeOne(this);
     if (makeChecksumHeader(checksumType, checksum) == _item->_checksumHeader) {
         // No download necessary, just update fs and journal metadata
         qCDebug(lcPropagateDownload) << _item->_file << "remote and local checksum match";