Merge pull request #12450 from Chocobo1/noBuffer

Avoid holding encoded resume data in memory

src/base/bittorrent/private/resumedatasavingmanager.cpp

@@ -28,11 +28,15 @@
 
 #include "resumedatasavingmanager.h"
 
+#include <libtorrent/bencode.hpp>
+#include <libtorrent/entry.hpp>
+
 #include <QByteArray>
 #include <QSaveFile>
 
 #include "base/logger.h"
 #include "base/utils/fs.h"
+#include "base/utils/io.h"
 
 ResumeDataSavingManager::ResumeDataSavingManager(const QString &resumeFolderPath)
     : m_resumeDataDir(resumeFolderPath)
@@ -50,6 +54,24 @@ void ResumeDataSavingManager::save(const QString &filename, const QByteArray &da
     }
 }
 
+void ResumeDataSavingManager::save(const QString &filename, const std::shared_ptr<lt::entry> &data) const
+{
+    const QString filepath = m_resumeDataDir.absoluteFilePath(filename);
+
+    QSaveFile file {filepath};
+    if (!file.open(QIODevice::WriteOnly)) {
+        LogMsg(tr("Couldn't save data to '%1'. Error: %2")
+            .arg(filepath, file.errorString()), Log::CRITICAL);
+        return;
+    }
+
+    lt::bencode(Utils::IO::FileDeviceOutputIterator {file}, *data);
+    if ((file.error() != QFileDevice::NoError) || !file.commit()) {
+        LogMsg(tr("Couldn't save data to '%1'. Error: %2")
+            .arg(filepath, file.errorString()), Log::CRITICAL);
+    }
+}
+
 void ResumeDataSavingManager::remove(const QString &filename) const
 {
     const QString filepath = m_resumeDataDir.absoluteFilePath(filename);

src/base/bittorrent/private/resumedatasavingmanager.h

@@ -28,6 +28,10 @@
 
 #pragma once
 
+#include <memory>
+
+#include <libtorrent/fwd.hpp>
+
 #include <QDir>
 #include <QObject>
 
@@ -43,8 +47,9 @@ public:
 
 public slots:
     void save(const QString &filename, const QByteArray &data) const;
+    void save(const QString &filename, const std::shared_ptr<lt::entry> &data) const;
     void remove(const QString &filename) const;
 
 private:
-    QDir m_resumeDataDir;
+    const QDir m_resumeDataDir;
 };

src/base/bittorrent/session.cpp

@@ -40,25 +40,6 @@
 #include <iphlpapi.h>
 #endif
 
-#include <QDebug>
-#include <QDir>
-#include <QFile>
-#include <QHostAddress>
-#include <QNetworkAddressEntry>
-#include <QNetworkConfigurationManager>
-#include <QNetworkInterface>
-#include <QRegularExpression>
-#include <QString>
-#include <QThread>
-#include <QTimer>
-#include <QUuid>
-
-#ifdef Q_OS_WIN
-// TODO: Remove together with fixBrokenSavePath()
-#define NEED_TO_FIX_BROKEN_PATH
-#include <QSaveFile>
-#endif
-
 #include <libtorrent/alert_types.hpp>
 #include <libtorrent/bdecode.hpp>
 #include <libtorrent/bencode.hpp>
@@ -78,6 +59,25 @@
 #include <libtorrent/read_resume_data.hpp>
 #endif
 
+#include <QDebug>
+#include <QDir>
+#include <QFile>
+#include <QHostAddress>
+#include <QNetworkAddressEntry>
+#include <QNetworkConfigurationManager>
+#include <QNetworkInterface>
+#include <QRegularExpression>
+#include <QString>
+#include <QThread>
+#include <QTimer>
+#include <QUuid>
+
+#ifdef Q_OS_WIN
+// TODO: Remove together with fixBrokenSavePath()
+#define NEED_TO_FIX_BROKEN_PATH
+#include <QSaveFile>
+#endif
+
 #include "base/algorithm.h"
 #include "base/exceptions.h"
 #include "base/global.h"
@@ -2587,6 +2587,7 @@ void Session::saveResumeData()
 
 void Session::saveTorrentsQueue()
 {
+    // store hash in textual representation
     QMap<int, QString> queue; // Use QMap since it should be ordered by key
     for (const TorrentHandle *torrent : asConst(torrents())) {
         // We require actual (non-cached) queue position here!
@@ -2596,6 +2597,7 @@ void Session::saveTorrentsQueue()
     }
 
     QByteArray data;
+    data.reserve(((InfoHash::length() * 2) + 1) * queue.size());
     for (const QString &hash : asConst(queue))
         data += (hash.toLatin1() + '\n');
 
@@ -3989,25 +3991,20 @@ void Session::handleTorrentFinished(TorrentHandle *const torrent)
         emit allTorrentsFinished();
 }
 
-void Session::handleTorrentResumeDataReady(TorrentHandle *const torrent, const lt::entry &data)
+void Session::handleTorrentResumeDataReady(TorrentHandle *const torrent, const std::shared_ptr<lt::entry> &data)
 {
     --m_numResumeData;
 
     // Separated thread is used for the blocking IO which results in slow processing of many torrents.
-    // Encoding data in parallel while doing IO saves time. Copying lt::entry objects around
+    // Copying lt::entry objects around isn't cheap.
-    // isn't cheap too.
-
-    QByteArray out;
-    out.reserve(1024 * 1024);  // most fastresume file sizes are under 1 MB
-    lt::bencode(std::back_inserter(out), data);
 
     const QString filename = QString::fromLatin1("%1.fastresume").arg(torrent->hash());
 #if (QT_VERSION >= QT_VERSION_CHECK(5, 10, 0))
     QMetaObject::invokeMethod(m_resumeDataSavingManager
-        , [this, filename, out]() { m_resumeDataSavingManager->save(filename, out); });
+        , [this, filename, data]() { m_resumeDataSavingManager->save(filename, data); });
 #else
-    QMetaObject::invokeMethod(m_resumeDataSavingManager, "save",
+    QMetaObject::invokeMethod(m_resumeDataSavingManager, "save"
-                              Q_ARG(QString, filename), Q_ARG(QByteArray, out));
+        , Q_ARG(QString, filename), Q_ARG(std::shared_ptr<lt::entry>, data));
 #endif
 }
 

src/base/bittorrent/session.h

@@ -30,6 +30,7 @@
 #ifndef BITTORRENT_SESSION_H
 #define BITTORRENT_SESSION_H
 
+#include <memory>
 #include <vector>
 
 #include <libtorrent/fwd.hpp>
@@ -459,7 +460,7 @@ namespace BitTorrent
         void handleTorrentTrackersChanged(TorrentHandle *const torrent);
         void handleTorrentUrlSeedsAdded(TorrentHandle *const torrent, const QVector<QUrl> &newUrlSeeds);
         void handleTorrentUrlSeedsRemoved(TorrentHandle *const torrent, const QVector<QUrl> &urlSeeds);
-        void handleTorrentResumeDataReady(TorrentHandle *const torrent, const lt::entry &data);
+        void handleTorrentResumeDataReady(TorrentHandle *const torrent, const std::shared_ptr<lt::entry> &data);
         void handleTorrentResumeDataFailed(TorrentHandle *const torrent);
         void handleTorrentTrackerReply(TorrentHandle *const torrent, const QString &trackerUrl);
         void handleTorrentTrackerWarning(TorrentHandle *const torrent, const QString &trackerUrl);

src/base/bittorrent/torrenthandle.cpp

@@ -30,6 +30,7 @@
 #include "torrenthandle.h"
 
 #include <algorithm>
+#include <memory>
 #include <type_traits>
 
 #ifdef Q_OS_WIN
@@ -1647,14 +1648,16 @@ void TorrentHandle::handleSaveResumeDataAlert(const lt::save_resume_data_alert *
 {
 #if (LIBTORRENT_VERSION_NUM < 10200)
     const bool useDummyResumeData = !(p && p->resume_data);
-    lt::entry dummyEntry;
+    auto resumeDataPtr = std::make_shared<lt::entry>(useDummyResumeData
-
+        ? lt::entry {}
-    lt::entry &resumeData = useDummyResumeData ? dummyEntry : *(p->resume_data);
+        : *(p->resume_data));
 #else
     const bool useDummyResumeData = !p;
-
+    auto resumeDataPtr = std::make_shared<lt::entry>(useDummyResumeData
-    lt::entry resumeData = useDummyResumeData ? lt::entry() : lt::write_resume_data(p->params);
+        ? lt::entry {}
+        : lt::write_resume_data(p->params));
 #endif
+    lt::entry &resumeData = *resumeDataPtr;
 
     updateStatus();
 
@@ -1697,7 +1700,7 @@ void TorrentHandle::handleSaveResumeDataAlert(const lt::save_resume_data_alert *
         resumeData["auto_managed"] = false;
     }
 
-    m_session->handleTorrentResumeDataReady(this, resumeData);
+    m_session->handleTorrentResumeDataReady(this, resumeDataPtr);
 }
 
 void TorrentHandle::handleSaveResumeDataFailedAlert(const lt::save_resume_data_failed_alert *p)

src/gui/mainwindow.cpp

@@ -118,8 +118,10 @@ namespace
     // Misc
     const QString KEY_DOWNLOAD_TRACKER_FAVICON = QStringLiteral(SETTINGS_KEY("DownloadTrackerFavicon"));
 
-    const int TIME_TRAY_BALLOON = 5000;
     const std::chrono::seconds PREVENT_SUSPEND_INTERVAL {60};
+#if !defined(Q_OS_MACOS)
+    const int TIME_TRAY_BALLOON = 5000;
+#endif
 
     // just a shortcut
     inline SettingsStorage *settings()