From a49397ed19f4dff0bddba555ddd715e3766c488b Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Mon, 25 Feb 2013 21:42:54 +0900
Subject: [PATCH] Fix infinite loop when size of downloadResults_ exceeds
 maxDownloadResult_

---
 src/RequestGroupMan.cc        |  3 +++
 test/RequestGroupManTest.cc   | 15 +++++++++++++++
 test/SessionSerializerTest.cc | 21 +--------------------
 test/TestUtil.cc              | 19 +++++++++++++++++++
 test/TestUtil.h               |  5 +++++
 5 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/src/RequestGroupMan.cc b/src/RequestGroupMan.cc
index e5cdc095..d232c5e0 100644
--- a/src/RequestGroupMan.cc
+++ b/src/RequestGroupMan.cc
@@ -883,11 +883,14 @@ void RequestGroupMan::addDownloadResult(const SharedHandle<DownloadResult>& dr)
   assert(rv);
   while(downloadResults_.size() > maxDownloadResult_){
     DownloadResultList::SeqType::iterator i = downloadResults_.begin();
+    // Save last encountered error code so that we can report it
+    // later.
     const SharedHandle<DownloadResult>& dr = (*i).second;
     if(dr->belongsTo == 0 && dr->result != error_code::FINISHED) {
       removedLastErrorResult_ = dr->result;
       ++removedErrorResult_;
     }
+    downloadResults_.pop_front();
   }
 }
 
diff --git a/test/RequestGroupManTest.cc b/test/RequestGroupManTest.cc
index ade0633b..5eda76de 100644
--- a/test/RequestGroupManTest.cc
+++ b/test/RequestGroupManTest.cc
@@ -34,6 +34,7 @@ class RequestGroupManTest : public CppUnit::TestFixture {
   CPPUNIT_TEST(testFillRequestGroupFromReserver);
   CPPUNIT_TEST(testFillRequestGroupFromReserver_uriParser);
   CPPUNIT_TEST(testInsertReservedGroup);
+  CPPUNIT_TEST(testAddDownloadResult);
   CPPUNIT_TEST_SUITE_END();
 private:
   SharedHandle<DownloadEngine> e_;
@@ -64,6 +65,7 @@ public:
   void testFillRequestGroupFromReserver();
   void testFillRequestGroupFromReserver_uriParser();
   void testInsertReservedGroup();
+  void testAddDownloadResult();
 };
 
 
@@ -289,4 +291,17 @@ void RequestGroupManTest::testInsertReservedGroup()
   CPPUNIT_ASSERT_EQUAL(rgs2[1]->getGID(), (*itr++).second->getGID());
 }
 
+void RequestGroupManTest::testAddDownloadResult()
+{
+  std::string uri = "http://example.org";
+  rgman_->setMaxDownloadResult(3);
+  rgman_->addDownloadResult(createDownloadResult(error_code::TIME_OUT, uri));
+  rgman_->addDownloadResult(createDownloadResult(error_code::FINISHED, uri));
+  rgman_->addDownloadResult(createDownloadResult(error_code::FINISHED, uri));
+  rgman_->addDownloadResult(createDownloadResult(error_code::FINISHED, uri));
+  rgman_->addDownloadResult(createDownloadResult(error_code::FINISHED, uri));
+  CPPUNIT_ASSERT_EQUAL(error_code::TIME_OUT,
+                       rgman_->getDownloadStat().getLastErrorResult());
+}
+
 } // namespace aria2
diff --git a/test/SessionSerializerTest.cc b/test/SessionSerializerTest.cc
index 1e61586c..f69860f5 100644
--- a/test/SessionSerializerTest.cc
+++ b/test/SessionSerializerTest.cc
@@ -5,6 +5,7 @@
 
 #include <cppunit/extensions/HelperMacros.h>
 
+#include "TestUtil.h"
 #include "RequestGroupMan.h"
 #include "array_fun.h"
 #include "download_helper.h"
@@ -29,26 +30,6 @@ public:
 
 CPPUNIT_TEST_SUITE_REGISTRATION(SessionSerializerTest);
 
-namespace {
-SharedHandle<DownloadResult> createDownloadResult
-(error_code::Value result, const std::string& uri)
-{
-  std::vector<std::string> uris;
-  uris.push_back(uri);
-  SharedHandle<FileEntry> entry(new FileEntry("/tmp/path", 1, 0, uris));
-  std::vector<SharedHandle<FileEntry> > entries;
-  entries.push_back(entry);
-  SharedHandle<DownloadResult> dr(new DownloadResult());
-  dr->gid = GroupId::create();
-  dr->fileEntries = entries;
-  dr->result = result;
-  dr->belongsTo = 0;
-  dr->inMemoryDownload = false;
-  dr->option = SharedHandle<Option>(new Option());
-  return dr;
-}
-} // namespace
-
 void SessionSerializerTest::testSave()
 {
 #if defined(ENABLE_BITTORRENT) && defined(ENABLE_METALINK)
diff --git a/test/TestUtil.cc b/test/TestUtil.cc
index e54e2aa0..0020f483 100644
--- a/test/TestUtil.cc
+++ b/test/TestUtil.cc
@@ -20,6 +20,7 @@
 #include "DownloadContext.h"
 #include "Option.h"
 #include "FileEntry.h"
+#include "DownloadResult.h"
 #ifdef ENABLE_MESSAGE_DIGEST
 # include "message_digest_helper.h"
 #endif // ENABLE_MESSAGE_DIGEST
@@ -144,4 +145,22 @@ SharedHandle<RequestGroup> createRequestGroup(int32_t pieceLength,
   return group;
 }
 
+SharedHandle<DownloadResult> createDownloadResult
+(error_code::Value result, const std::string& uri)
+{
+  std::vector<std::string> uris;
+  uris.push_back(uri);
+  SharedHandle<FileEntry> entry(new FileEntry("/tmp/path", 1, 0, uris));
+  std::vector<SharedHandle<FileEntry> > entries;
+  entries.push_back(entry);
+  SharedHandle<DownloadResult> dr(new DownloadResult());
+  dr->gid = GroupId::create();
+  dr->fileEntries = entries;
+  dr->result = result;
+  dr->belongsTo = 0;
+  dr->inMemoryDownload = false;
+  dr->option = SharedHandle<Option>(new Option());
+  return dr;
+}
+
 } // namespace aria2
diff --git a/test/TestUtil.h b/test/TestUtil.h
index 82a4663d..f23729f9 100644
--- a/test/TestUtil.h
+++ b/test/TestUtil.h
@@ -13,6 +13,7 @@ class MessageDigest;
 class RequestGroupMan;
 class RequestGroup;
 class Option;
+class DownloadResult;
 
 void createFile(const std::string& filename, size_t length);
 
@@ -70,4 +71,8 @@ SharedHandle<RequestGroup> createRequestGroup(int32_t pieceLength,
                                               const std::string& path,
                                               const std::string& uri,
                                               const SharedHandle<Option>& opt);
+
+SharedHandle<DownloadResult> createDownloadResult
+(error_code::Value result, const std::string& uri);
+
 } // namespace aria2