Fix infinite loop when size of downloadResults_ exceeds maxDownloadResult_

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();
   }
 }
 

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

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)

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

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