From 427577eed473b1880fdb2263be4cdad1eaaabfce Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Mon, 21 Jun 2010 14:02:51 +0000
Subject: [PATCH] 2010-06-21  Tatsuhiro Tsujikawa 
 <t-tujikawa@users.sourceforge.net>

	Fixed the bug that corrups file if segment returned from
	SegmetnMan::getCleanSegmentIfOwnerIsIdle() has writtenLength > 0.
	* src/DownloadCommand.cc
	* src/SegmentMan.cc
	* src/SegmentMan.h
	* test/SegmentManTest.cc
---
 ChangeLog              |  9 +++++++++
 src/DownloadCommand.cc | 10 ++++++++--
 src/SegmentMan.cc      |  2 +-
 src/SegmentMan.h       | 10 ++++------
 test/SegmentManTest.cc |  9 ---------
 5 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4aceba05..5e0eb9e9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2010-06-21  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
+
+	Fixed the bug that corrups file if segment returned from
+	SegmetnMan::getCleanSegmentIfOwnerIsIdle() has writtenLength > 0.
+	* src/DownloadCommand.cc
+	* src/SegmentMan.cc
+	* src/SegmentMan.h
+	* test/SegmentManTest.cc
+
 2010-06-21  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
 
 	Changed naming standards for class member variable: now it looks
diff --git a/src/DownloadCommand.cc b/src/DownloadCommand.cc
index 122cf26a..1bba9d04 100644
--- a/src/DownloadCommand.cc
+++ b/src/DownloadCommand.cc
@@ -315,10 +315,16 @@ bool DownloadCommand::prepareForNextSegment() {
       if(!tempSegment->complete()) {
         return prepareForRetry(0);
       }
-      SharedHandle<Segment> nextSegment =
-        getSegmentMan()->getCleanSegmentIfOwnerIsIdle
+      SharedHandle<Segment> nextSegment = getSegmentMan()->getSegment
         (getCuid(), tempSegment->getIndex()+1);
       if(nextSegment.isNull()) {
+        nextSegment = getSegmentMan()->getCleanSegmentIfOwnerIsIdle
+        (getCuid(), tempSegment->getIndex()+1);
+      }
+      if(nextSegment.isNull() || nextSegment->getWrittenLength() > 0) {
+        // If nextSegment->getWrittenLength() > 0, current socket must
+        // be closed because writing incoming data at
+        // nextSegment->getWrittenLength() corrupts file.
         return prepareForRetry(0);
       } else {
         getDownloadEngine()->addCommand(this);
diff --git a/src/SegmentMan.cc b/src/SegmentMan.cc
index ae17f8de..786fc3c0 100644
--- a/src/SegmentMan.cc
+++ b/src/SegmentMan.cc
@@ -236,7 +236,7 @@ SharedHandle<Segment> SegmentMan::getCleanSegmentIfOwnerIsIdle
       }
     }
   }
-  return checkoutSegment(cuid, pieceStorage_->getMissingPiece(index));
+  return SharedHandle<Segment>();
 }
 
 void SegmentMan::cancelSegment(const SharedHandle<Segment>& segment)
diff --git a/src/SegmentMan.h b/src/SegmentMan.h
index 15fe0ff1..0cdaa349 100644
--- a/src/SegmentMan.h
+++ b/src/SegmentMan.h
@@ -155,12 +155,10 @@ public:
    */
   SharedHandle<Segment> getSegment(cuid_t cuid, size_t index);
 
-  // Returns a segment whose index is index.  If the segment whose
-  // index is index is free, it is assigned to cuid and it is
-  // returned.  If it has already be assigned to another cuid, and if
-  // it is idle state and segment's written length is 0, then cancels
-  // the assignment and re-attach the segment to given cuid and the
-  // segment is returned. Otherwise returns null.
+  // Returns a currently used segment whose index is index and written
+  // length is 0.  The current owner(in idle state) of segment cancels
+  // the segment and cuid command acquires the ownership of the
+  // segment.  If no such segment exists, returns null.
   SharedHandle<Segment> getCleanSegmentIfOwnerIsIdle(cuid_t cuid, size_t index);
 
   /**
diff --git a/test/SegmentManTest.cc b/test/SegmentManTest.cc
index 691a599b..80a4f9d8 100644
--- a/test/SegmentManTest.cc
+++ b/test/SegmentManTest.cc
@@ -194,15 +194,6 @@ void SegmentManTest::testGetCleanSegmentIfOwnerIsIdle()
   CPPUNIT_ASSERT(segmentMan_->getCleanSegmentIfOwnerIsIdle(5, 0).isNull());
   // Segment::updateWrittenLength != 0
   CPPUNIT_ASSERT(segmentMan_->getCleanSegmentIfOwnerIsIdle(5, 1).isNull());
-
-  // Test with UnknownLengthPieceStorage
-  SharedHandle<DownloadContext> dctx(new DownloadContext(1024, 0, "aria2"));
-  SharedHandle<UnknownLengthPieceStorage> ps
-    (new UnknownLengthPieceStorage(dctx, option_.get()));
-  segmentMan_.reset(new SegmentMan(option_.get(), dctx, ps));
-  
-  CPPUNIT_ASSERT(!segmentMan_->getCleanSegmentIfOwnerIsIdle(1, 0).isNull());
-
 }
 
 } // namespace aria2