diff --git a/ChangeLog b/ChangeLog index 0499123e..f2c353cf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,17 @@ +2010-07-11 Tatsuhiro Tsujikawa + + 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 After change request to faster one, wait at least 10 seconds. diff --git a/src/AbstractCommand.cc b/src/AbstractCommand.cc index 0c7c59e0..e48b7954 100644 --- a/src/AbstractCommand.cc +++ b/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 = 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()); diff --git a/src/DownloadCommand.cc b/src/DownloadCommand.cc index 829a7fd4..fe82521c 100644 --- a/src/DownloadCommand.cc +++ b/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 = @@ -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 && diff --git a/src/DownloadCommand.h b/src/DownloadCommand.h index 9e55784a..128b6bf7 100644 --- a/src/DownloadCommand.h +++ b/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& req, diff --git a/src/FtpDownloadCommand.cc b/src/FtpDownloadCommand.cc index 4d20c3c7..a72d98a0 100644 --- a/src/FtpDownloadCommand.cc +++ b/src/FtpDownloadCommand.cc @@ -88,4 +88,9 @@ bool FtpDownloadCommand::prepareForNextSegment() } } +off_t FtpDownloadCommand::getRequestEndOffset() const +{ + return getFileEntry()->getLength(); +} + } // namespace aria2 diff --git a/src/FtpDownloadCommand.h b/src/FtpDownloadCommand.h index dd6e7196..0380f624 100644 --- a/src/FtpDownloadCommand.h +++ b/src/FtpDownloadCommand.h @@ -48,6 +48,7 @@ private: SharedHandle ctrlSocket_; protected: virtual bool prepareForNextSegment(); + virtual off_t getRequestEndOffset() const; public: FtpDownloadCommand(cuid_t cuid, const SharedHandle& req, diff --git a/src/HttpDownloadCommand.cc b/src/HttpDownloadCommand.cc index 5727ed6f..deed69d7 100644 --- a/src/HttpDownloadCommand.cc +++ b/src/HttpDownloadCommand.cc @@ -129,4 +129,9 @@ bool HttpDownloadCommand::prepareForNextSegment() { } } +off_t HttpDownloadCommand::getRequestEndOffset() const +{ + return httpResponse_->getHttpHeader()->getRange()->getEndByte()+1; +} + } // namespace aria2 diff --git a/src/HttpDownloadCommand.h b/src/HttpDownloadCommand.h index 1f9ebcf2..07d976a7 100644 --- a/src/HttpDownloadCommand.h +++ b/src/HttpDownloadCommand.h @@ -48,6 +48,7 @@ private: SharedHandle httpConnection_; protected: virtual bool prepareForNextSegment(); + virtual off_t getRequestEndOffset() const; public: HttpDownloadCommand(cuid_t cuid, const SharedHandle& req,