From 12659c74a8ad48eafc3704da1809e84f1ded96ec Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Tue, 25 Oct 2011 21:44:04 +0900
Subject: [PATCH] Don't copy Option in RequestGroup ctor.

Copy on receive is not a practice in aria2 source code.
---
 src/Metalink2RequestGroup.cc | 17 +++++++++--------
 src/RequestGroup.cc          |  2 +-
 src/RequestGroup.h           |  1 -
 src/TrackerWatcherCommand.cc | 19 ++++++++++---------
 src/download_helper.cc       | 21 +++++++++++++--------
 src/util.h                   |  6 ++++++
 test/BtDependencyTest.cc     |  5 +++--
 test/RequestGroupManTest.cc  | 13 +++++++------
 test/RpcMethodTest.cc        |  8 ++++----
 9 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/src/Metalink2RequestGroup.cc b/src/Metalink2RequestGroup.cc
index 21f1f289..7f97e894 100644
--- a/src/Metalink2RequestGroup.cc
+++ b/src/Metalink2RequestGroup.cc
@@ -147,23 +147,23 @@ void
 Metalink2RequestGroup::createRequestGroup
 (std::vector<SharedHandle<RequestGroup> >& groups,
  const std::vector<SharedHandle<MetalinkEntry> >& entries,
- const SharedHandle<Option>& option)
+ const SharedHandle<Option>& optionTemplate)
 {
   if(entries.empty()) {
     A2_LOG_NOTICE(EX_NO_RESULT_WITH_YOUR_PREFS);
     return;
   }
   std::vector<int32_t> selectIndexes =
-    util::parseIntRange(option->get(PREF_SELECT_FILE)).flush();
+    util::parseIntRange(optionTemplate->get(PREF_SELECT_FILE)).flush();
   std::sort(selectIndexes.begin(), selectIndexes.end());
   std::vector<std::string> locations;
-  if(option->defined(PREF_METALINK_LOCATION)) {
-    util::split(util::toLower(option->get(PREF_METALINK_LOCATION)),
+  if(optionTemplate->defined(PREF_METALINK_LOCATION)) {
+    util::split(util::toLower(optionTemplate->get(PREF_METALINK_LOCATION)),
                 std::back_inserter(locations), ",", true);
   }
   std::string preferredProtocol;
-  if(option->get(PREF_METALINK_PREFERRED_PROTOCOL) != V_NONE) {
-    preferredProtocol = option->get(PREF_METALINK_PREFERRED_PROTOCOL);
+  if(optionTemplate->get(PREF_METALINK_PREFERRED_PROTOCOL) != V_NONE) {
+    preferredProtocol = optionTemplate->get(PREF_METALINK_PREFERRED_PROTOCOL);
   }
   std::vector<SharedHandle<MetalinkEntry> > selectedEntries;
   selectedEntries.reserve(entries.size());
@@ -205,7 +205,7 @@ Metalink2RequestGroup::createRequestGroup
       uris.push_back(metaurl);
       {
         std::vector<SharedHandle<RequestGroup> > result;
-        createRequestGroupForUri(result, option, uris,
+        createRequestGroupForUri(result, optionTemplate, uris,
                                  /* ignoreForceSequential = */true,
                                  /* ignoreLocalPath = */true);
         if(!uris.empty()) {
@@ -229,6 +229,7 @@ Metalink2RequestGroup::createRequestGroup
       }
     }
 #endif // ENABLE_BITTORRENT
+    SharedHandle<Option> option = util::copy(optionTemplate);
     SharedHandle<RequestGroup> rg(new RequestGroup(option));
     SharedHandle<DownloadContext> dctx;
     if(mes.size() == 1) {
@@ -311,7 +312,7 @@ Metalink2RequestGroup::createRequestGroup
     }
     rg->setDownloadContext(dctx);
     rg->setPauseRequested(option->getAsBool(PREF_PAUSE));
-    removeOneshotOption(rg->getOption());
+    removeOneshotOption(option);
     // remove "metalink" from Accept Type list to avoid loop in
     // tranparent metalink
     util::removeMetalinkContentTypes(rg);
diff --git a/src/RequestGroup.cc b/src/RequestGroup.cc
index b0f52e1c..4d001ab0 100644
--- a/src/RequestGroup.cc
+++ b/src/RequestGroup.cc
@@ -124,7 +124,7 @@ a2_gid_t RequestGroup::gidCounter_ = 0;
 
 RequestGroup::RequestGroup(const SharedHandle<Option>& option)
   : gid_(newGID()),
-    option_(new Option(*option.get())),
+    option_(option),
     numConcurrentCommand_(option->getAsInt(PREF_SPLIT)),
     numStreamConnection_(0),
     numStreamCommand_(0),
diff --git a/src/RequestGroup.h b/src/RequestGroup.h
index 86651e1c..7458cb96 100644
--- a/src/RequestGroup.h
+++ b/src/RequestGroup.h
@@ -196,7 +196,6 @@ private:
 
   bool isCheckIntegrityReady() const;
 public:
-  // The copy of option is stored in RequestGroup object.
   RequestGroup(const SharedHandle<Option>& option);
 
   ~RequestGroup();
diff --git a/src/TrackerWatcherCommand.cc b/src/TrackerWatcherCommand.cc
index fd4f3465..5f5a4db8 100644
--- a/src/TrackerWatcherCommand.cc
+++ b/src/TrackerWatcherCommand.cc
@@ -215,7 +215,8 @@ TrackerWatcherCommand::createRequestGroup(const std::string& uri)
 {
   std::vector<std::string> uris;
   uris.push_back(uri);
-  SharedHandle<RequestGroup> rg(new RequestGroup(getOption()));
+  SharedHandle<Option> option = util::copy(getOption());
+  SharedHandle<RequestGroup> rg(new RequestGroup(option));
   if(backupTrackerIsAvailable(requestGroup_->getDownloadContext())) {
     A2_LOG_DEBUG("This is multi-tracker announce.");
   } else {
@@ -224,19 +225,19 @@ TrackerWatcherCommand::createRequestGroup(const std::string& uri)
   rg->setNumConcurrentCommand(1);
   // If backup tracker is available, try 2 times for each tracker
   // and if they all fails, then try next one.
-  rg->getOption()->put(PREF_MAX_TRIES, "2");
+  option->put(PREF_MAX_TRIES, "2");
   // TODO When dry-run mode becomes available in BitTorrent, set
   // PREF_DRY_RUN=false too.
-  rg->getOption()->put(PREF_USE_HEAD, A2_V_FALSE);
+  option->put(PREF_USE_HEAD, A2_V_FALSE);
   // Setting tracker timeouts
-  rg->setTimeout(rg->getOption()->getAsInt(PREF_BT_TRACKER_TIMEOUT));
-  rg->getOption()->put(PREF_CONNECT_TIMEOUT,
-                       rg->getOption()->get(PREF_BT_TRACKER_CONNECT_TIMEOUT));
-  rg->getOption()->put(PREF_REUSE_URI, A2_V_FALSE);
-  rg->getOption()->put(PREF_SELECT_LEAST_USED_HOST, A2_V_FALSE);
+  rg->setTimeout(option->getAsInt(PREF_BT_TRACKER_TIMEOUT));
+  option->put(PREF_CONNECT_TIMEOUT,
+              option->get(PREF_BT_TRACKER_CONNECT_TIMEOUT));
+  option->put(PREF_REUSE_URI, A2_V_FALSE);
+  option->put(PREF_SELECT_LEAST_USED_HOST, A2_V_FALSE);
   static const std::string TRACKER_ANNOUNCE_FILE("[tracker.announce]");
   SharedHandle<DownloadContext> dctx
-    (new DownloadContext(getOption()->getAsInt(PREF_PIECE_LENGTH),
+    (new DownloadContext(option->getAsInt(PREF_PIECE_LENGTH),
                          0,
                          TRACKER_ANNOUNCE_FILE));
   dctx->getFileEntries().front()->setUris(uris);
diff --git a/src/download_helper.cc b/src/download_helper.cc
index 52530a01..1581aca8 100644
--- a/src/download_helper.cc
+++ b/src/download_helper.cc
@@ -110,9 +110,11 @@ void splitURI(std::vector<std::string>& result,
 
 namespace {
 SharedHandle<RequestGroup> createRequestGroup
-(const SharedHandle<Option>& option, const std::vector<std::string>& uris,
+(const SharedHandle<Option>& optionTemplate,
+ const std::vector<std::string>& uris,
  bool useOutOption = false)
 {
+  SharedHandle<Option> option = util::copy(optionTemplate);
   SharedHandle<RequestGroup> rg(new RequestGroup(option));
   SharedHandle<DownloadContext> dctx
     (new DownloadContext
@@ -135,7 +137,7 @@ SharedHandle<RequestGroup> createRequestGroup
 #endif // ENABLE_MESSAGE_DIGEST
   rg->setDownloadContext(dctx);
   rg->setPauseRequested(option->getAsBool(PREF_PAUSE));
-  removeOneshotOption(rg->getOption());
+  removeOneshotOption(option);
   return rg;
 }
 } // namespace
@@ -161,11 +163,12 @@ SharedHandle<MetadataInfo> createMetadataInfoDataOnly()
 namespace {
 SharedHandle<RequestGroup>
 createBtRequestGroup(const std::string& torrentFilePath,
-                     const SharedHandle<Option>& option,
+                     const SharedHandle<Option>& optionTemplate,
                      const std::vector<std::string>& auxUris,
                      const std::string& torrentData = "",
                      bool adjustAnnounceUri = true)
 {
+  SharedHandle<Option> option = util::copy(optionTemplate);
   SharedHandle<RequestGroup> rg(new RequestGroup(option));
   SharedHandle<DownloadContext> dctx(new DownloadContext());
   if(torrentData.empty()) {
@@ -194,16 +197,18 @@ createBtRequestGroup(const std::string& torrentFilePath,
   // Remove "metalink" from Accept Type list to avoid server from
   // responding Metalink file for web-seeding URIs.
   util::removeMetalinkContentTypes(rg);
-  removeOneshotOption(rg->getOption());
+  removeOneshotOption(option);
   return rg;
 }
 } // namespace
 
 namespace {
 SharedHandle<RequestGroup>
-createBtMagnetRequestGroup(const std::string& magnetLink,
-                           const SharedHandle<Option>& option)
+createBtMagnetRequestGroup
+(const std::string& magnetLink,
+ const SharedHandle<Option>& optionTemplate)
 {
+  SharedHandle<Option> option = util::copy(optionTemplate);
   SharedHandle<RequestGroup> rg(new RequestGroup(option));
   SharedHandle<DownloadContext> dctx
     (new DownloadContext(METADATA_PIECE_SIZE, 0,
@@ -215,7 +220,7 @@ createBtMagnetRequestGroup(const std::string& magnetLink,
   bittorrent::loadMagnet(magnetLink, dctx);
   SharedHandle<TorrentAttribute> torrentAttrs =
     bittorrent::getTorrentAttrs(dctx);
-  bittorrent::adjustAnnounceUri(torrentAttrs, rg->getOption());
+  bittorrent::adjustAnnounceUri(torrentAttrs, option);
   dctx->getFirstFileEntry()->setPath(torrentAttrs->name);
   rg->setDownloadContext(dctx);
   rg->clearPostDownloadHandler();
@@ -227,7 +232,7 @@ createBtMagnetRequestGroup(const std::string& magnetLink,
   rg->setMetadataInfo(createMetadataInfo(magnetLink));
   rg->markInMemoryDownload();
   rg->setPauseRequested(option->getAsBool(PREF_PAUSE));
-  removeOneshotOption(rg->getOption());
+  removeOneshotOption(option);
   return rg;
 }
 } // namespace
diff --git a/src/util.h b/src/util.h
index bc973e92..d51348fb 100644
--- a/src/util.h
+++ b/src/util.h
@@ -514,6 +514,12 @@ nextParam
   return std::make_pair(end, false);
 }
 
+template<typename T>
+SharedHandle<T> copy(const SharedHandle<T>& a)
+{
+  return SharedHandle<T>(new T(*a.get()));
+}
+
 } // namespace util
 
 } // namespace aria2
diff --git a/test/BtDependencyTest.cc b/test/BtDependencyTest.cc
index f44b5f76..aa0f4465 100644
--- a/test/BtDependencyTest.cc
+++ b/test/BtDependencyTest.cc
@@ -18,6 +18,7 @@
 #include "ByteArrayDiskWriter.h"
 #include "MockPieceStorage.h"
 #include "prefs.h"
+#include "util.h"
 
 namespace aria2 {
 
@@ -37,7 +38,7 @@ class BtDependencyTest:public CppUnit::TestFixture {
 
   SharedHandle<RequestGroup> createDependant(const SharedHandle<Option>& option)
   {
-    SharedHandle<RequestGroup> dependant(new RequestGroup(option));
+    SharedHandle<RequestGroup> dependant(new RequestGroup(util::copy(option)));
     SharedHandle<DownloadContext> dctx
       (new DownloadContext(0, 0, "/tmp/outfile.path"));
     std::vector<std::string> uris;
@@ -55,7 +56,7 @@ class BtDependencyTest:public CppUnit::TestFixture {
    const std::string& torrentFile,
    int64_t length)
   {
-    SharedHandle<RequestGroup> dependee(new RequestGroup(option));
+    SharedHandle<RequestGroup> dependee(new RequestGroup(util::copy(option)));
     SharedHandle<DownloadContext> dctx
       (new DownloadContext(1024*1024, length, torrentFile));
     dependee->setDownloadContext(dctx);
diff --git a/test/RequestGroupManTest.cc b/test/RequestGroupManTest.cc
index 2dbff95c..22f9c9bb 100644
--- a/test/RequestGroupManTest.cc
+++ b/test/RequestGroupManTest.cc
@@ -15,6 +15,7 @@
 #include "File.h"
 #include "array_fun.h"
 #include "RecoverableException.h"
+#include "util.h"
 
 namespace aria2 {
 
@@ -48,8 +49,8 @@ CPPUNIT_TEST_SUITE_REGISTRATION( RequestGroupManTest );
 
 void RequestGroupManTest::testIsSameFileBeingDownloaded()
 {
-  SharedHandle<RequestGroup> rg1(new RequestGroup(option_));
-  SharedHandle<RequestGroup> rg2(new RequestGroup(option_));
+  SharedHandle<RequestGroup> rg1(new RequestGroup(util::copy(option_)));
+  SharedHandle<RequestGroup> rg2(new RequestGroup(util::copy(option_)));
 
   SharedHandle<DownloadContext> dctx1
     (new DownloadContext(0, 0, "aria2.tar.bz2"));
@@ -117,10 +118,10 @@ void RequestGroupManTest::testLoadServerStat()
 void RequestGroupManTest::testChangeReservedGroupPosition()
 {
   SharedHandle<RequestGroup> gs[] = {
-    SharedHandle<RequestGroup>(new RequestGroup(option_)),
-    SharedHandle<RequestGroup>(new RequestGroup(option_)),
-    SharedHandle<RequestGroup>(new RequestGroup(option_)),
-    SharedHandle<RequestGroup>(new RequestGroup(option_))
+    SharedHandle<RequestGroup>(new RequestGroup(util::copy(option_))),
+    SharedHandle<RequestGroup>(new RequestGroup(util::copy(option_))),
+    SharedHandle<RequestGroup>(new RequestGroup(util::copy(option_))),
+    SharedHandle<RequestGroup>(new RequestGroup(util::copy(option_)))
   };
   std::vector<SharedHandle<RequestGroup> > groups(vbegin(gs), vend(gs));
   RequestGroupMan rm(groups, 0, option_.get());
diff --git a/test/RpcMethodTest.cc b/test/RpcMethodTest.cc
index 4e686fa1..8987755f 100644
--- a/test/RpcMethodTest.cc
+++ b/test/RpcMethodTest.cc
@@ -752,11 +752,11 @@ void RpcMethodTest::testGatherProgressCommon()
   SharedHandle<DownloadContext> dctx(new DownloadContext(0, 0,"aria2.tar.bz2"));
   std::string uris[] = { "http://localhost/aria2.tar.bz2" };
   dctx->getFirstFileEntry()->addUris(vbegin(uris), vend(uris));
-  SharedHandle<RequestGroup> group(new RequestGroup(option_));
+  SharedHandle<RequestGroup> group(new RequestGroup(util::copy(option_)));
   group->setDownloadContext(dctx);
   std::vector<SharedHandle<RequestGroup> > followedBy;
   for(int i = 0; i < 2; ++i) {
-    followedBy.push_back(SharedHandle<RequestGroup>(new RequestGroup(option_)));
+    followedBy.push_back(SharedHandle<RequestGroup>(new RequestGroup(util::copy(option_))));
   }
 
   group->followedBy(followedBy.begin(), followedBy.end());
@@ -844,9 +844,9 @@ void RpcMethodTest::testGatherBitTorrentMetadata()
 void RpcMethodTest::testChangePosition()
 {
   e_->getRequestGroupMan()->addReservedGroup
-    (SharedHandle<RequestGroup>(new RequestGroup(option_)));
+    (SharedHandle<RequestGroup>(new RequestGroup(util::copy(option_))));
   e_->getRequestGroupMan()->addReservedGroup
-    (SharedHandle<RequestGroup>(new RequestGroup(option_)));
+    (SharedHandle<RequestGroup>(new RequestGroup(util::copy(option_))));
 
   ChangePositionRpcMethod m;
   RpcRequest req(ChangePositionRpcMethod::getMethodName(), List::g());