Selaa lähdekoodia

2010-07-11 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>

	Fixed the bug that segments are not filled to
	Request::getMaxPipelinedRequest().
	Make sure that trailing data of transfer encoding is read propery,
	after file data is received.
	* src/AbstractCommand.cc
	* src/DownloadCommand.cc
	* src/DownloadCommand.h
	* src/FtpDownloadCommand.cc
	* src/FtpDownloadCommand.h
	* src/HttpDownloadCommand.cc
	* src/HttpDownloadCommand.h
Tatsuhiro Tsujikawa 15 vuotta sitten
vanhempi
commit
f3b097b5af

+ 14 - 0
ChangeLog

@@ -1,3 +1,17 @@
+2010-07-11  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
+
+	Fixed the bug that segments are not filled to
+	Request::getMaxPipelinedRequest().
+	Make sure that trailing data of transfer encoding is read propery,
+	after file data is received.
+	* src/AbstractCommand.cc
+	* src/DownloadCommand.cc
+	* src/DownloadCommand.h
+	* src/FtpDownloadCommand.cc
+	* src/FtpDownloadCommand.h
+	* src/HttpDownloadCommand.cc
+	* src/HttpDownloadCommand.h
+
 2010-07-11  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
 
 	After change request to faster one, wait at least 10 seconds.

+ 21 - 4
src/AbstractCommand.cc

@@ -160,26 +160,43 @@ bool AbstractCommand::execute() {
         segments_.clear();
         getSegmentMan()->getInFlightSegment(segments_, getCuid());
         if(!req_.isNull() && segments_.empty()) {
+          // TODO make this out side of socket check if.
+
           // This command previously has assigned segments, but it is
-          // canceled. So discard current request chain.
+          // canceled. So discard current request chain.  Plus, if no
+          // segment is available when http pipelining is used.
           if(getLogger()->debug()) {
             getLogger()->debug("CUID#%s - It seems previously assigned segments"
                                " are canceled. Restart.",
                                util::itos(getCuid()).c_str());
           }
+          // Request::isPipeliningEnabled() == true means aria2
+          // accessed the remote server and discovered that the server
+          // supports pipelining.
+          if(!req_.isNull() && req_->isPipeliningEnabled()) {
+            e_->poolSocket(req_, createProxyRequest(), socket_);
+          }
           return prepareForRetry(0);
         }
         if(req_.isNull() || req_->getMaxPipelinedRequest() == 1 ||
+           // Why the following condition is necessary? That's because
+           // For single file download, SegmentMan::getSegment(cuid)
+           // is more efficient.
            getDownloadContext()->getFileEntries().size() == 1) {
-          if(segments_.empty()) {
+          size_t maxSegments = req_.isNull()?1:req_->getMaxPipelinedRequest();
+          while(segments_.size() < maxSegments) {
             SharedHandle<Segment> segment =
               getSegmentMan()->getSegment(getCuid());
-            if(!segment.isNull()) {
+            if(segment.isNull()) {
+              break;
+            } else {
               segments_.push_back(segment);
             }
           }
           if(segments_.empty()) {
-            // TODO socket could be pooled here if pipelining is enabled...
+            // TODO socket could be pooled here if pipelining is
+            // enabled...  Hmm, I don't think if pipelining is enabled
+            // it does not go here.
             if(getLogger()->info()) {
               getLogger()->info(MSG_NO_SEGMENT_AVAILABLE,
                                 util::itos(getCuid()).c_str());

+ 32 - 11
src/DownloadCommand.cc

@@ -135,6 +135,17 @@ bool DownloadCommand::executeInternal() {
   } else {
     bufSize = BUFSIZE;
   }
+  // It is possible that segment is completed but we have some bytes
+  // of stream to read. For example, chunked encoding has "0"+CRLF
+  // after data. After we read data(at this moment segment is
+  // completed), we need another 3bytes(or more if it has extension).
+  if(bufSize == 0 &&
+     ((!transferEncodingDecoder_.isNull() &&
+       !transferEncodingDecoder_->finished()) ||
+      (!contentEncodingDecoder_.isNull() &&
+       !contentEncodingDecoder_->finished()))) {
+    bufSize = 1;
+  }
   getSocket()->readData(buf_, bufSize);
 
   const SharedHandle<DiskAdaptor>& diskAdaptor =
@@ -187,17 +198,27 @@ bool DownloadCommand::executeInternal() {
               !getSocket()->wantRead() && !getSocket()->wantWrite()) {
       segmentPartComplete = true;
     }
-  } else if(!transferEncodingDecoder_.isNull() &&
-            (segment->complete() ||
-             segment->getPositionToWrite() == getFileEntry()->getLastOffset())){
-    // In this case, transferEncodingDecoder is used and
-    // Content-Length is known.
-    segmentPartComplete = true;
-  } else if((transferEncodingDecoder_.isNull() ||
-             transferEncodingDecoder_->finished()) &&
-            (contentEncodingDecoder_.isNull() ||
-             contentEncodingDecoder_->finished())) {
-    segmentPartComplete = true;
+  } else {
+    off_t loff = getFileEntry()->gtoloff(segment->getPositionToWrite());
+    if(!transferEncodingDecoder_.isNull() &&
+       ((loff == getRequestEndOffset() && transferEncodingDecoder_->finished())
+        || loff < getRequestEndOffset()) &&
+       (segment->complete() ||
+        segment->getPositionToWrite() == getFileEntry()->getLastOffset())) {
+      // In this case, transferEncodingDecoder is used and
+      // Content-Length is known.  We check
+      // transferEncodingDecoder_->finished() only if the requested
+      // end offset equals to written position in file local offset;
+      // in other words, data in the requested ranage is all received.
+      // If requested end offset is greater than this segment, then
+      // transferEncodingDecoder_ is not finished in this segment.
+      segmentPartComplete = true;
+    } else if((transferEncodingDecoder_.isNull() ||
+               transferEncodingDecoder_->finished()) &&
+              (contentEncodingDecoder_.isNull() ||
+               contentEncodingDecoder_->finished())) {
+      segmentPartComplete = true;
+    }
   }
 
   if(!segmentPartComplete && bufSize == 0 &&

+ 3 - 0
src/DownloadCommand.h

@@ -74,6 +74,9 @@ protected:
   virtual bool executeInternal();
 
   virtual bool prepareForNextSegment();
+
+  // This is file local offset
+  virtual off_t getRequestEndOffset() const = 0;
 public:
   DownloadCommand(cuid_t cuid,
                   const SharedHandle<Request>& req,

+ 5 - 0
src/FtpDownloadCommand.cc

@@ -88,4 +88,9 @@ bool FtpDownloadCommand::prepareForNextSegment()
   }
 }
 
+off_t FtpDownloadCommand::getRequestEndOffset() const
+{
+  return getFileEntry()->getLength();
+}
+
 } // namespace aria2

+ 1 - 0
src/FtpDownloadCommand.h

@@ -48,6 +48,7 @@ private:
   SharedHandle<SocketCore> ctrlSocket_;
 protected:
   virtual bool prepareForNextSegment();
+  virtual off_t getRequestEndOffset() const;
 public:
   FtpDownloadCommand(cuid_t cuid,
                      const SharedHandle<Request>& req,

+ 5 - 0
src/HttpDownloadCommand.cc

@@ -129,4 +129,9 @@ bool HttpDownloadCommand::prepareForNextSegment() {
   }
 }
 
+off_t HttpDownloadCommand::getRequestEndOffset() const
+{
+  return httpResponse_->getHttpHeader()->getRange()->getEndByte()+1;
+}
+
 } // namespace aria2

+ 1 - 0
src/HttpDownloadCommand.h

@@ -48,6 +48,7 @@ private:
   SharedHandle<HttpConnection> httpConnection_;
 protected:
   virtual bool prepareForNextSegment();
+  virtual off_t getRequestEndOffset() const;
 public:
   HttpDownloadCommand(cuid_t cuid,
                       const SharedHandle<Request>& req,