From 8897d7ec702fca9780033db42db48cced2580112 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Fri, 6 May 2016 18:23:44 +0900
Subject: [PATCH] Restart active download to apply previously not applicable
 options

Previously, we categorized options that can be used in
aria2.changeOption RPC method into 2 categories.  The options in one
category can be applied on the fly, meaning that download continues to
be active while applying options.  Another category includes options
which are only applicable when downloads are waiting or paused.

In this change, when active download is ordered to change options
which only applicable in waiting or paused state, it is now paused,
and then automatically restarted.  Although we have limited number of
download concurrency, the pause and restart is done atomically, and
the download is inserted at the front of the queue, it is picked up
immediately if the concurrency regulation allows.
---
 src/Option.cc          |  6 +++++
 src/Option.h           |  2 ++
 src/RequestGroup.cc    |  7 ++++++
 src/RequestGroup.h     | 18 +++++++++++++++
 src/RequestGroupMan.cc | 28 ++++++++++++++++++-----
 src/RpcMethod.cc       | 51 +++++++++++++++++++++++++++++++++++++-----
 src/RpcMethod.h        |  3 ++-
 src/RpcMethodImpl.cc   | 16 +++++++++++--
 src/bitfield.h         |  3 +--
 9 files changed, 119 insertions(+), 15 deletions(-)

diff --git a/src/Option.cc b/src/Option.cc
index 93d81a1c..e2af7017 100644
--- a/src/Option.cc
+++ b/src/Option.cc
@@ -190,4 +190,10 @@ void Option::setParent(const std::shared_ptr<Option>& parent)
 
 const std::shared_ptr<Option>& Option::getParent() const { return parent_; }
 
+bool Option::emptyLocal() const
+{
+  size_t dst;
+  return !bitfield::getFirstSetBitIndex(dst, use_, use_.size() * 8);
+}
+
 } // namespace aria2
diff --git a/src/Option.h b/src/Option.h
index 1e00f98a..76193084 100644
--- a/src/Option.h
+++ b/src/Option.h
@@ -92,6 +92,8 @@ public:
   // Sets parent Option object for this object.
   void setParent(const std::shared_ptr<Option>& parent);
   const std::shared_ptr<Option>& getParent() const;
+  // Returns true if there is no option stored.
+  bool emptyLocal() const;
 };
 
 } // namespace aria2
diff --git a/src/RequestGroup.cc b/src/RequestGroup.cc
index 85b75a45..4a75e3a1 100644
--- a/src/RequestGroup.cc
+++ b/src/RequestGroup.cc
@@ -957,6 +957,8 @@ void RequestGroup::setForceHaltRequested(bool f, HaltReason haltReason)
 
 void RequestGroup::setPauseRequested(bool f) { pauseRequested_ = f; }
 
+void RequestGroup::setRestartRequested(bool f) { restartRequested_ = f; }
+
 void RequestGroup::releaseRuntimeResource(DownloadEngine* e)
 {
 #ifdef ENABLE_BITTORRENT
@@ -1279,4 +1281,9 @@ bool RequestGroup::isSeeder() const
 #endif // !ENABLE_BITTORRENT
 }
 
+void RequestGroup::setPendingOption(std::shared_ptr<Option> option)
+{
+  pendingOption_ = std::move(option);
+}
+
 } // namespace aria2
diff --git a/src/RequestGroup.h b/src/RequestGroup.h
index 0d94c5da..8b7862eb 100644
--- a/src/RequestGroup.h
+++ b/src/RequestGroup.h
@@ -96,6 +96,9 @@ private:
 
   std::shared_ptr<Option> option_;
 
+  // options applied on restart
+  std::shared_ptr<Option> pendingOption_;
+
   std::shared_ptr<SegmentMan> segmentMan_;
 
   std::shared_ptr<DownloadContext> downloadContext_;
@@ -178,6 +181,11 @@ private:
 
   bool pauseRequested_;
 
+  // restartRequested_ indicates that this download should be
+  // restarted.  Usually, it is used with pauseRequested_ to stop
+  // download first.
+  bool restartRequested_;
+
   // This flag just indicates that the downloaded file is not saved disk but
   // just sits in memory.
   bool inMemoryDownload_;
@@ -345,6 +353,10 @@ public:
 
   bool isPauseRequested() const { return pauseRequested_; }
 
+  void setRestartRequested(bool f);
+
+  bool isRestartRequested() const { return restartRequested_; }
+
   void dependsOn(const std::shared_ptr<Dependency>& dep);
 
   bool isDependencyResolved();
@@ -500,6 +512,12 @@ public:
 
   // Returns true if this download is now seeding.
   bool isSeeder() const;
+
+  void setPendingOption(std::shared_ptr<Option> option);
+  const std::shared_ptr<Option>& getPendingOption() const
+  {
+    return pendingOption_;
+  }
 };
 
 } // namespace aria2
diff --git a/src/RequestGroupMan.cc b/src/RequestGroupMan.cc
index 1eaf75e3..19ee63ad 100644
--- a/src/RequestGroupMan.cc
+++ b/src/RequestGroupMan.cc
@@ -84,6 +84,7 @@
 #include "array_fun.h"
 #include "OpenedFileCounter.h"
 #include "wallclock.h"
+#include "RpcMethodImpl.h"
 #ifdef ENABLE_BITTORRENT
 #include "bittorrent_helper.h"
 #endif // ENABLE_BITTORRENT
@@ -369,8 +370,10 @@ public:
       try {
         group->closeFile();
         if (group->isPauseRequested()) {
-          A2_LOG_NOTICE(fmt(_("Download GID#%s paused"),
-                            GroupId::toHex(group->getGID()).c_str()));
+          if (!group->isRestartRequested()) {
+            A2_LOG_NOTICE(fmt(_("Download GID#%s paused"),
+                              GroupId::toHex(group->getGID()).c_str()));
+          }
           group->saveControlFile();
         }
         else if (group->downloadFinished() &&
@@ -433,9 +436,20 @@ public:
         reservedGroups_.push_front(group->getGID(), group);
         group->releaseRuntimeResource(e_);
         group->setForceHaltRequested(false);
-        util::executeHookByOptName(group, e_->getOption(),
-                                   PREF_ON_DOWNLOAD_PAUSE);
-        notifyDownloadEvent(EVENT_ON_DOWNLOAD_PAUSE, group);
+
+        auto pendingOption = group->getPendingOption();
+        if (pendingOption) {
+          changeOption(group, *pendingOption, e_);
+        }
+
+        if (group->isRestartRequested()) {
+          group->setPauseRequested(false);
+        }
+        else {
+          util::executeHookByOptName(group, e_->getOption(),
+                                     PREF_ON_DOWNLOAD_PAUSE);
+          notifyDownloadEvent(EVENT_ON_DOWNLOAD_PAUSE, group);
+        }
         // TODO Should we have to prepend spend uris to remaining uris
         // in case PREF_REUSE_URI is disabled?
       }
@@ -445,6 +459,10 @@ public:
         executeStopHook(group, e_->getOption(), dr->result);
         group->releaseRuntimeResource(e_);
       }
+
+      group->setRestartRequested(false);
+      group->setPendingOption(nullptr);
+
       return true;
     }
     else {
diff --git a/src/RpcMethod.cc b/src/RpcMethod.cc
index ecfaa955..eaf1c4b1 100644
--- a/src/RpcMethod.cc
+++ b/src/RpcMethod.cc
@@ -147,12 +147,53 @@ void RpcMethod::gatherRequestOption(Option* option, const Dict* optionsDict)
   }
 }
 
-void RpcMethod::gatherChangeableOption(Option* option, const Dict* optionsDict)
+void RpcMethod::gatherChangeableOption(Option* option, Option* pendingOption,
+                                       const Dict* optionsDict)
 {
-  if (optionsDict) {
-    gatherOption(optionsDict->begin(), optionsDict->end(),
-                 std::mem_fn(&OptionHandler::getChangeOption), option,
-                 optionParser_);
+  if (!optionsDict) {
+    return;
+  }
+
+  auto first = optionsDict->begin();
+  auto last = optionsDict->end();
+
+  for (; first != last; ++first) {
+    const auto& optionName = (*first).first;
+    auto pref = option::k2p(optionName);
+    auto handler = optionParser_->find(pref);
+    if (!handler) {
+      // Just ignore the unacceptable options in this context.
+      continue;
+    }
+
+    Option* dst = nullptr;
+    if (handler->getChangeOption()) {
+      dst = option;
+    }
+    else if (handler->getChangeOptionForReserved()) {
+      dst = pendingOption;
+    }
+
+    if (!dst) {
+      continue;
+    }
+
+    const auto opval = downcast<String>((*first).second);
+    if (opval) {
+      handler->parse(*dst, opval->s());
+    }
+    else if (handler->getCumulative()) {
+      // header and index-out option can take array as value
+      const auto oplist = downcast<List>((*first).second);
+      if (oplist) {
+        for (auto& elem : *oplist) {
+          const auto opval = downcast<String>(elem);
+          if (opval) {
+            handler->parse(*dst, opval->s());
+          }
+        }
+      }
+    }
   }
 }
 
diff --git a/src/RpcMethod.h b/src/RpcMethod.h
index 8496d62f..eb6af79f 100644
--- a/src/RpcMethod.h
+++ b/src/RpcMethod.h
@@ -75,7 +75,8 @@ protected:
 
   void gatherRequestOption(Option* option, const Dict* optionsDict);
 
-  void gatherChangeableOption(Option* option, const Dict* optionDict);
+  void gatherChangeableOption(Option* option, Option* pendingOption,
+                              const Dict* optionDict);
 
   void gatherChangeableOptionForReserved(Option* option,
                                          const Dict* optionsDict);
diff --git a/src/RpcMethodImpl.cc b/src/RpcMethodImpl.cc
index 058dda82..15746097 100644
--- a/src/RpcMethodImpl.cc
+++ b/src/RpcMethodImpl.cc
@@ -1113,10 +1113,22 @@ std::unique_ptr<ValueBase> ChangeOptionRpcMethod::process(const RpcRequest& req,
 
   a2_gid_t gid = str2Gid(gidParam);
   auto group = e->getRequestGroupMan()->findGroup(gid);
-  Option option;
   if (group) {
+    Option option;
+    std::shared_ptr<Option> pendingOption;
     if (group->getState() == RequestGroup::STATE_ACTIVE) {
-      gatherChangeableOption(&option, optsParam);
+      pendingOption = std::make_shared<Option>();
+      gatherChangeableOption(&option, pendingOption.get(), optsParam);
+      if (!pendingOption->emptyLocal()) {
+        group->setPendingOption(pendingOption);
+        // pauseRequestGroup() may fail if group has been told to
+        // stop/pause already.  In that case, we can still apply the
+        // pending options on pause.
+        if (pauseRequestGroup(group, false, false)) {
+          group->setRestartRequested(true);
+          e->setRefreshInterval(std::chrono::milliseconds(0));
+        }
+      }
     }
     else {
       gatherChangeableOptionForReserved(&option, optsParam);
diff --git a/src/bitfield.h b/src/bitfield.h
index e628e602..9928d8bd 100644
--- a/src/bitfield.h
+++ b/src/bitfield.h
@@ -143,8 +143,7 @@ size_t countSetBitSlow(const Array& bitfield, size_t nbits)
 void flipBit(unsigned char* data, size_t length, size_t bitIndex);
 
 // Stores first set bit index of bitfield to index.  bitfield contains
-// nbits. Returns true if missing bit index is found. Otherwise
-// returns false.
+// nbits. Returns true if set bit is found. Otherwise returns false.
 template <typename Array>
 bool getFirstSetBitIndex(size_t& index, const Array& bitfield, size_t nbits)
 {