Disable --deferred-input when --save-session is used together

With --deferred-input=true, aria2 only reads input file to fill active
download slots, while keeping input file open.  Meanwhile,
--save-session saves all download info inside memory, but this does
not take into account of unread item in input file.  This will lead to
lose input data in saved session file.  Also current BufferedFile
implementation used to read/write input/output file take a lock on
Windows.  This effectively prevents session serializer from writing
session data to the same file which is still kept open because of
--deferred-input.  See GH-493

6 files changed, 56 insertions, 12 deletions

diff --git a/doc/manual-src/en/aria2c.rst b/doc/manual-src/en/aria2c.rst
index a75cb7be..8a2657a3 100644
--- a/doc/manual-src/en/aria2c.rst
+++ b/doc/manual-src/en/aria2c.rst
@@ -1207,6 +1207,11 @@ Advanced Options
   and options at startup.
   Default: ``false``
 
+  .. Warning::
+
+    :option:`--deferred-input` option will be disabled when
+    :option:`--save-session` is used together.
+
 .. option:: --disable-ipv6[=true|false]
 
   Disable IPv6. This is useful if you have to use broken DNS and want
diff --git a/src/Context.cc b/src/Context.cc
index 6a3aac83..70bbb4e5 100644
--- a/src/Context.cc
+++ b/src/Context.cc
@@ -278,16 +278,15 @@ Context::Context(bool standalone,
   // command-line. If they are left, because op is used as a template
   // for new RequestGroup(such as created in RPC command), they causes
   // unintentional effect.
-  for(auto i = op; i; i = i->getParent()) {
-    i->remove(PREF_OUT);
-    i->remove(PREF_FORCE_SEQUENTIAL);
-    i->remove(PREF_INPUT_FILE);
-    i->remove(PREF_INDEX_OUT);
-    i->remove(PREF_SELECT_FILE);
-    i->remove(PREF_PAUSE);
-    i->remove(PREF_CHECKSUM);
-    i->remove(PREF_GID);
-  }
+  op->remove(PREF_OUT);
+  op->remove(PREF_FORCE_SEQUENTIAL);
+  op->remove(PREF_INPUT_FILE);
+  op->remove(PREF_INDEX_OUT);
+  op->remove(PREF_SELECT_FILE);
+  op->remove(PREF_PAUSE);
+  op->remove(PREF_CHECKSUM);
+  op->remove(PREF_GID);
+
   if(standalone &&
      !op->getAsBool(PREF_ENABLE_RPC) && requestGroups.empty() &&
      !uriListParser) {
diff --git a/src/Option.cc b/src/Option.cc
index 633e9281..4c46fe92 100644
--- a/src/Option.cc
+++ b/src/Option.cc
@@ -147,12 +147,20 @@ double Option::getAsDouble(PrefPtr pref) const {
   }
 }
 
-void Option::remove(PrefPtr pref)
+void Option::removeLocal(PrefPtr pref)
 {
   unsetBit(use_, pref);
   table_[pref->i].clear();
 }
 
+void Option::remove(PrefPtr pref)
+{
+  removeLocal(pref);
+  if (parent_) {
+    parent_->remove(pref);
+  }
+}
+
 void Option::clear()
 {
   std::fill(use_.begin(), use_.end(), 0);
diff --git a/src/Option.h b/src/Option.h
index 7cbf7648..b9b4cfab 100644
--- a/src/Option.h
+++ b/src/Option.h
@@ -76,6 +76,8 @@ public:
   double getAsDouble(PrefPtr pref) const;
   // Removes |pref| from this object. This function does not modify
   // parent_.
+  void removeLocal(PrefPtr pref);
+  // Removes |pref| from this object from all option hierarchy.
   void remove(PrefPtr pref);
   // Removes all option values from this object. This function does
   // not modify parent_.
diff --git a/src/option_processing.cc b/src/option_processing.cc
index 45a01650..78560020 100644
--- a/src/option_processing.cc
+++ b/src/option_processing.cc
@@ -59,6 +59,7 @@
 #include "BufferedFile.h"
 #include "console.h"
 #include "array_fun.h"
+#include "LogFactory.h"
 #ifndef HAVE_DAEMON
 #include "daemon.h"
 #endif // !HAVE_DAEMON
@@ -331,6 +332,11 @@ error_code::Value option_processing(Option& op, bool standalone,
       return error_code::UNKNOWN_ERROR;
     }
   }
+  if (op.getAsBool(PREF_DEFERRED_INPUT) && op.defined(PREF_SAVE_SESSION)) {
+    A2_LOG_WARN("--deferred-input is disabled because of the presence of "
+                "--save-session");
+    op.remove(PREF_DEFERRED_INPUT);
+  }
   return error_code::FINISHED;
 }
 
diff --git a/test/OptionTest.cc b/test/OptionTest.cc
index f7ce58f2..b648923a 100644
--- a/test/OptionTest.cc
+++ b/test/OptionTest.cc
@@ -18,6 +18,7 @@ class OptionTest:public CppUnit::TestFixture {
   CPPUNIT_TEST(testBlank);
   CPPUNIT_TEST(testMerge);
   CPPUNIT_TEST(testParent);
+  CPPUNIT_TEST(testRemove);
   CPPUNIT_TEST_SUITE_END();
 private:
 
@@ -32,6 +33,7 @@ public:
   void testBlank();
   void testMerge();
   void testParent();
+  void testRemove();
 };
 
 
@@ -118,9 +120,31 @@ void OptionTest::testParent()
   CPPUNIT_ASSERT(child.defined(PREF_TIMEOUT));
   CPPUNIT_ASSERT(child.definedLocal(PREF_TIMEOUT));
   CPPUNIT_ASSERT_EQUAL(std::string("200"), child.get(PREF_TIMEOUT));
-  child.remove(PREF_TIMEOUT);
+  child.removeLocal(PREF_TIMEOUT);
   CPPUNIT_ASSERT(child.defined(PREF_TIMEOUT));
   CPPUNIT_ASSERT(!child.definedLocal(PREF_TIMEOUT));
 }
 
+void OptionTest::testRemove()
+{
+  Option child;
+  auto parent = std::make_shared<Option>();
+  child.setParent(parent);
+
+  child.put(PREF_DIR, "foo");
+  child.put(PREF_TIMEOUT, "200");
+  parent->put(PREF_DIR, "bar");
+  parent->put(PREF_TIMEOUT, "400");
+
+  child.remove(PREF_DIR);
+
+  CPPUNIT_ASSERT(!child.defined(PREF_DIR));
+
+  child.removeLocal(PREF_TIMEOUT);
+
+  CPPUNIT_ASSERT(!child.definedLocal(PREF_TIMEOUT));
+  CPPUNIT_ASSERT(child.defined(PREF_TIMEOUT));
+  CPPUNIT_ASSERT(parent->defined(PREF_TIMEOUT));
+}
+
 } // namespace aria2