summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--ChangeLog12
-rw-r--r--src/MultiDiskAdaptor.cc119
-rw-r--r--src/MultiDiskAdaptor.h5
-rw-r--r--src/MultiFileAllocationIterator.cc2
-rw-r--r--test/MultiDiskAdaptorTest.cc2
-rw-r--r--test/MultiFileAllocationIteratorTest.cc112
6 files changed, 179 insertions, 73 deletions
diff --git a/ChangeLog b/ChangeLog
index fa1c50e0..bef852bb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,17 @@
2008-09-07 Tatsuhiro Tsujikawa <tujikawa at rednoah dot com>
+ Fixed the bug that DiskWriterEntry is not created when its
+ FileEntry.isRequested() is false and it doesn't share a piece with
+ other FileEntries that are requested. This bug causes segmentation fault
+ in the end.
+ * src/MultiDiskAdaptor.cc
+ * src/MultiDiskAdaptor.h
+ * src/MultiFileAllocationIterator.cc
+ * test/MultiDiskAdaptorTest.cc
+ * test/MultiFileAllocationIteratorTest.cc
+
+2008-09-07 Tatsuhiro Tsujikawa <tujikawa at rednoah dot com>
+
Fixed the bug that exception is thrown when MultiDiskAdaptor::size() is
called if the number of file entries are greater than max open files.
* src/AbstractSingleDiskAdaptor.cc
diff --git a/src/MultiDiskAdaptor.cc b/src/MultiDiskAdaptor.cc
index 2df1b68d..a2377ace 100644
--- a/src/MultiDiskAdaptor.cc
+++ b/src/MultiDiskAdaptor.cc
@@ -45,11 +45,13 @@
#include "Logger.h"
#include "SimpleRandomizer.h"
#include <algorithm>
+#include <cassert>
namespace aria2 {
DiskWriterEntry::DiskWriterEntry(const SharedHandle<FileEntry>& fileEntry):
- fileEntry(fileEntry), _open(false), _directIO(false) {}
+ fileEntry(fileEntry), _open(false), _directIO(false),
+ _needsFileAllocation(false) {}
DiskWriterEntry::~DiskWriterEntry() {}
@@ -60,29 +62,35 @@ std::string DiskWriterEntry::getFilePath(const std::string& topDir) const
void DiskWriterEntry::initAndOpenFile(const std::string& topDir)
{
- diskWriter->initAndOpenFile(getFilePath(topDir), fileEntry->getLength());
- if(_directIO) {
- diskWriter->enableDirectIO();
+ if(!diskWriter.isNull()) {
+ diskWriter->initAndOpenFile(getFilePath(topDir), fileEntry->getLength());
+ if(_directIO) {
+ diskWriter->enableDirectIO();
+ }
+ _open = true;
}
- _open = true;
}
void DiskWriterEntry::openFile(const std::string& topDir)
{
- diskWriter->openFile(getFilePath(topDir), fileEntry->getLength());
- if(_directIO) {
- diskWriter->enableDirectIO();
+ if(!diskWriter.isNull()) {
+ diskWriter->openFile(getFilePath(topDir), fileEntry->getLength());
+ if(_directIO) {
+ diskWriter->enableDirectIO();
+ }
+ _open = true;
}
- _open = true;
}
void DiskWriterEntry::openExistingFile(const std::string& topDir)
{
- diskWriter->openExistingFile(getFilePath(topDir), fileEntry->getLength());
- if(_directIO) {
- diskWriter->enableDirectIO();
+ if(!diskWriter.isNull()) {
+ diskWriter->openExistingFile(getFilePath(topDir), fileEntry->getLength());
+ if(_directIO) {
+ diskWriter->enableDirectIO();
+ }
+ _open = true;
}
- _open = true;
}
bool DiskWriterEntry::isOpen() const
@@ -105,6 +113,7 @@ bool DiskWriterEntry::fileExists(const std::string& topDir)
uint64_t DiskWriterEntry::size() const
{
+ assert(!diskWriter.isNull());
return diskWriter->size();
}
@@ -144,6 +153,16 @@ void DiskWriterEntry::disableDirectIO()
_directIO = false;
}
+bool DiskWriterEntry::needsFileAllocation() const
+{
+ return _needsFileAllocation;
+}
+
+void DiskWriterEntry::needsFileAllocation(bool f)
+{
+ _needsFileAllocation = f;
+}
+
MultiDiskAdaptor::MultiDiskAdaptor():
pieceLength(0),
_maxOpenFiles(DEFAULT_MAX_OPEN_FILES),
@@ -153,13 +172,10 @@ MultiDiskAdaptor::~MultiDiskAdaptor() {}
static SharedHandle<DiskWriterEntry> createDiskWriterEntry
(const SharedHandle<FileEntry>& fileEntry,
- DiskWriterFactory& dwFactory,
- bool directIOAllowed)
+ bool needsFileAllocation)
{
SharedHandle<DiskWriterEntry> entry(new DiskWriterEntry(fileEntry));
- entry->setDiskWriter(dwFactory.newDiskWriter());
- entry->getDiskWriter()->setDirectIOAllowed(directIOAllowed);
-
+ entry->needsFileAllocation(needsFileAllocation);
return entry;
}
@@ -172,49 +188,42 @@ void MultiDiskAdaptor::resetDiskWriterEntries()
return;
}
- DefaultDiskWriterFactory dwFactory;
- if(pieceLength == 0) {
- for(std::deque<SharedHandle<FileEntry> >::const_iterator itr =
- fileEntries.begin(); itr != fileEntries.end(); ++itr) {
- if((*itr)->isRequested()) {
- diskWriterEntries.push_back
- (createDiskWriterEntry(*itr, dwFactory, _directIOAllowed));
- }
- }
- } else {
- std::deque<SharedHandle<FileEntry> >::const_iterator done = fileEntries.begin();
- for(std::deque<SharedHandle<FileEntry> >::const_iterator itr =
- fileEntries.begin(); itr != fileEntries.end();) {
- if(!(*itr)->isRequested()) {
+ for(std::deque<SharedHandle<FileEntry> >::const_iterator i =
+ fileEntries.begin(); i != fileEntries.end(); ++i) {
+ diskWriterEntries.push_back
+ (createDiskWriterEntry(*i, (*i)->isRequested()));
+ }
+
+ // TODO Currently, pieceLength == 0 is used for unit testing only.
+ if(pieceLength > 0) {
+ std::deque<SharedHandle<DiskWriterEntry> >::iterator done =
+ diskWriterEntries.begin();
+ for(std::deque<SharedHandle<DiskWriterEntry> >::iterator itr =
+ diskWriterEntries.begin(); itr != diskWriterEntries.end();) {
+ if(!(*itr)->getFileEntry()->isRequested()) {
++itr;
continue;
}
- off_t pieceStartOffset = ((*itr)->getOffset()/pieceLength)*pieceLength;
- std::deque<SharedHandle<DiskWriterEntry> >::iterator insertionPoint =
- diskWriterEntries.end();
-
- if(itr != fileEntries.begin()) {
- for(std::deque<SharedHandle<FileEntry> >::const_iterator i = itr-1;
- i != done; --i) {
- if((uint64_t)pieceStartOffset < (*i)->getOffset()+(*i)->getLength()) {
- insertionPoint = diskWriterEntries.insert
- (insertionPoint,
- createDiskWriterEntry(*i, dwFactory, _directIOAllowed));
+ off_t pieceStartOffset =
+ ((*itr)->getFileEntry()->getOffset()/pieceLength)*pieceLength;
+ if(itr != diskWriterEntries.begin()) {
+ for(std::deque<SharedHandle<DiskWriterEntry> >::iterator i =
+ itr-1; i != done; --i) {
+ const SharedHandle<FileEntry>& fileEntry = (*i)->getFileEntry();
+ if((uint64_t)pieceStartOffset <
+ fileEntry->getOffset()+fileEntry->getLength()) {
+ (*i)->needsFileAllocation(true);
} else {
break;
}
}
}
- diskWriterEntries.push_back
- (createDiskWriterEntry(*itr, dwFactory, _directIOAllowed));
-
++itr;
- for(; itr != fileEntries.end(); ++itr) {
- if((*itr)->getOffset() < pieceStartOffset+pieceLength) {
- diskWriterEntries.push_back
- (createDiskWriterEntry(*itr, dwFactory, _directIOAllowed));
+ for(; itr != diskWriterEntries.end(); ++itr) {
+ if((*itr)->getFileEntry()->getOffset() < pieceStartOffset+pieceLength) {
+ (*itr)->needsFileAllocation(true);
} else {
break;
}
@@ -223,6 +232,16 @@ void MultiDiskAdaptor::resetDiskWriterEntries()
done = itr-1;
}
}
+ DefaultDiskWriterFactory dwFactory;
+ for(std::deque<SharedHandle<DiskWriterEntry> >::iterator i =
+ diskWriterEntries.begin(); i != diskWriterEntries.end(); ++i) {
+ if((*i)->needsFileAllocation() || (*i)->fileExists(getTopDirPath())) {
+ logger->debug("Creating DiskWriter for filename=%s",
+ (*i)->getFilePath(getTopDirPath()).c_str());
+ (*i)->setDiskWriter(dwFactory.newDiskWriter());
+ (*i)->getDiskWriter()->setDirectIOAllowed(_directIOAllowed);
+ }
+ }
}
std::string MultiDiskAdaptor::getTopDirPath() const
diff --git a/src/MultiDiskAdaptor.h b/src/MultiDiskAdaptor.h
index bc7500f2..4533b7ce 100644
--- a/src/MultiDiskAdaptor.h
+++ b/src/MultiDiskAdaptor.h
@@ -49,6 +49,7 @@ private:
SharedHandle<DiskWriter> diskWriter;
bool _open;
bool _directIO;
+ bool _needsFileAllocation;
public:
DiskWriterEntry(const SharedHandle<FileEntry>& fileEntry);
@@ -87,6 +88,10 @@ public:
// Additionally, if diskWriter is opened, diskWriter->disableDirectIO() is
// called.
void disableDirectIO();
+
+ bool needsFileAllocation() const;
+
+ void needsFileAllocation(bool f);
};
typedef SharedHandle<DiskWriterEntry> DiskWriterEntryHandle;
diff --git a/src/MultiFileAllocationIterator.cc b/src/MultiFileAllocationIterator.cc
index 1f541361..7fcb7b3e 100644
--- a/src/MultiFileAllocationIterator.cc
+++ b/src/MultiFileAllocationIterator.cc
@@ -61,7 +61,7 @@ void MultiFileAllocationIterator::allocateChunk()
_diskAdaptor->openIfNot(entry, &DiskWriterEntry::openFile,
_diskAdaptor->getTopDirPath());
entry->enableDirectIO();
- if(entry->size() < fileEntry->getLength()) {
+ if(entry->needsFileAllocation() && entry->size() < fileEntry->getLength()) {
// Calling private function of MultiDiskAdaptor.
_fileAllocationIterator.reset
(new SingleFileAllocationIterator(entry->getDiskWriter().get(),
diff --git a/test/MultiDiskAdaptorTest.cc b/test/MultiDiskAdaptorTest.cc
index 295bce55..0cc38dde 100644
--- a/test/MultiDiskAdaptorTest.cc
+++ b/test/MultiDiskAdaptorTest.cc
@@ -158,6 +158,7 @@ void MultiDiskAdaptorTest::testCutTrailingGarbage()
adaptor.setTopDir(topDir);
adaptor.setFileEntries(fileEntries);
adaptor.setMaxOpenFiles(1);
+ adaptor.setPieceLength(1);
adaptor.openFile();
@@ -191,6 +192,7 @@ void MultiDiskAdaptorTest::testSize()
adaptor.setTopDir(topDir);
adaptor.setFileEntries(fileEntries);
adaptor.setMaxOpenFiles(1);
+ adaptor.setPieceLength(1);
adaptor.openFile();
diff --git a/test/MultiFileAllocationIteratorTest.cc b/test/MultiFileAllocationIteratorTest.cc
index 40c12646..288df5e2 100644
--- a/test/MultiFileAllocationIteratorTest.cc
+++ b/test/MultiFileAllocationIteratorTest.cc
@@ -4,6 +4,8 @@
#include "FileEntry.h"
#include "Exception.h"
#include "array_fun.h"
+#include "TestUtil.h"
+#include "DiskWriter.h"
#include <algorithm>
#include <iostream>
#include <cppunit/extensions/HelperMacros.h>
@@ -50,37 +52,93 @@ void MultiFileAllocationIteratorTest::testMakeDiskWriterEntries()
fs[6]->setRequested(false); // file7
fs[8]->setRequested(false); // file9
fs[9]->setRequested(false); // fileA
+
+ std::string topDir = "top";
+ std::string storeDir = "/tmp/aria2_MultiFileAllocationIteratorTest"
+ "_testMakeDiskWriterEntries";
+ std::string prefix = storeDir+"/"+topDir;
+
+ // create empty file4
+ createFile(prefix+std::string("/file4"), 0);
- std::string storeDir = "/tmp/aria2_MultiFileAllocationIteratorTest_testMakeDiskWriterEntries";
SharedHandle<MultiDiskAdaptor> diskAdaptor(new MultiDiskAdaptor());
- diskAdaptor->setFileEntries(std::deque<SharedHandle<FileEntry> >(&fs[0], &fs[arrayLength(fs)]));
+ diskAdaptor->setFileEntries
+ (std::deque<SharedHandle<FileEntry> >(&fs[0], &fs[arrayLength(fs)]));
diskAdaptor->setPieceLength(1024);
diskAdaptor->setStoreDir(storeDir);
+ diskAdaptor->setTopDir(topDir);
diskAdaptor->openFile();
SharedHandle<MultiFileAllocationIterator> itr
- (dynamic_pointer_cast<MultiFileAllocationIterator>(diskAdaptor->fileAllocationIterator()));
+ (dynamic_pointer_cast<MultiFileAllocationIterator>
+ (diskAdaptor->fileAllocationIterator()));
DiskWriterEntries entries = itr->getDiskWriterEntries();
- std::sort(entries.begin(), entries.end());
-
- CPPUNIT_ASSERT_EQUAL((size_t)8, entries.size());
-
- CPPUNIT_ASSERT_EQUAL(storeDir+std::string("/file1"), entries[0]->getFilePath(storeDir));
- CPPUNIT_ASSERT_EQUAL(storeDir+std::string("/file2"), entries[1]->getFilePath(storeDir));
- CPPUNIT_ASSERT_EQUAL(storeDir+std::string("/file3"), entries[2]->getFilePath(storeDir));
- CPPUNIT_ASSERT_EQUAL(storeDir+std::string("/file6"), entries[3]->getFilePath(storeDir));
- CPPUNIT_ASSERT_EQUAL(storeDir+std::string("/file7"), entries[4]->getFilePath(storeDir));
- CPPUNIT_ASSERT_EQUAL(storeDir+std::string("/file8"), entries[5]->getFilePath(storeDir));
- CPPUNIT_ASSERT_EQUAL(storeDir+std::string("/file9"), entries[6]->getFilePath(storeDir));
- CPPUNIT_ASSERT_EQUAL(storeDir+std::string("/fileB"), entries[7]->getFilePath(storeDir));
+ CPPUNIT_ASSERT_EQUAL((size_t)11, entries.size());
+
+ // file1
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/file1"),
+ entries[0]->getFilePath(prefix));
+ CPPUNIT_ASSERT(entries[0]->needsFileAllocation());
+ CPPUNIT_ASSERT(!entries[0]->getDiskWriter().isNull());
+ // file2
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/file2"),
+ entries[1]->getFilePath(prefix));
+ CPPUNIT_ASSERT(entries[1]->needsFileAllocation());
+ CPPUNIT_ASSERT(!entries[1]->getDiskWriter().isNull());
+ // file3
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/file3"),
+ entries[2]->getFilePath(prefix));
+ CPPUNIT_ASSERT(entries[2]->needsFileAllocation());
+ CPPUNIT_ASSERT(!entries[2]->getDiskWriter().isNull());
+ // file4, diskWriter is not null, because file exists.
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/file4"),
+ entries[3]->getFilePath(prefix));
+ CPPUNIT_ASSERT(!entries[3]->needsFileAllocation());
+ CPPUNIT_ASSERT(!entries[3]->getDiskWriter().isNull());
+ // file5
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/file5"),
+ entries[4]->getFilePath(prefix));
+ CPPUNIT_ASSERT(!entries[4]->needsFileAllocation());
+ CPPUNIT_ASSERT(entries[4]->getDiskWriter().isNull());
+ // file6
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/file6"),
+ entries[5]->getFilePath(prefix));
+ CPPUNIT_ASSERT(entries[5]->needsFileAllocation());
+ CPPUNIT_ASSERT(!entries[5]->getDiskWriter().isNull());
+ // file7
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/file7"),
+ entries[6]->getFilePath(prefix));
+ CPPUNIT_ASSERT(entries[6]->needsFileAllocation());
+ CPPUNIT_ASSERT(!entries[6]->getDiskWriter().isNull());
+ // file8
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/file8"),
+ entries[7]->getFilePath(prefix));
+ CPPUNIT_ASSERT(entries[7]->needsFileAllocation());
+ CPPUNIT_ASSERT(!entries[7]->getDiskWriter().isNull());
+ // file9
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/file9"),
+ entries[8]->getFilePath(prefix));
+ CPPUNIT_ASSERT(entries[8]->needsFileAllocation());
+ CPPUNIT_ASSERT(!entries[8]->getDiskWriter().isNull());
+ // fileA
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/fileA"),
+ entries[9]->getFilePath(prefix));
+ CPPUNIT_ASSERT(!entries[9]->needsFileAllocation());
+ CPPUNIT_ASSERT(entries[9]->getDiskWriter().isNull());
+ // fileB
+ CPPUNIT_ASSERT_EQUAL(prefix+std::string("/fileB"),
+ entries[10]->getFilePath(prefix));
+ CPPUNIT_ASSERT(entries[10]->needsFileAllocation());
+ CPPUNIT_ASSERT(!entries[10]->getDiskWriter().isNull());
}
void MultiFileAllocationIteratorTest::testAllocate()
{
std::string dir = "/tmp";
std::string topDir = "aria2_MultiFileAllocationIteratorTest_testAllocate";
+ std::string prefix = dir+"/"+topDir;
std::string fname1 = "file1";
std::string fname2 = "file2";
std::string fname3 = "file3";
@@ -98,6 +156,7 @@ void MultiFileAllocationIteratorTest::testAllocate()
SharedHandle<MultiDiskAdaptor> diskAdaptor(new MultiDiskAdaptor());
diskAdaptor->setStoreDir(dir);
diskAdaptor->setTopDir(topDir);
+ diskAdaptor->setPieceLength(1);
int64_t offset = 0;
SharedHandle<FileEntry> fileEntry1(new FileEntry(fname1,
@@ -139,20 +198,29 @@ void MultiFileAllocationIteratorTest::testAllocate()
fs.push_back(fileEntry6);
diskAdaptor->setFileEntries(fs);
+
+ File(prefix+"/"+fname1).remove();
+ File(prefix+"/"+fname2).remove();
+ File(prefix+"/"+fname3).remove();
+ File(prefix+"/"+fname4).remove();
+ File(prefix+"/"+fname5).remove();
+ File(prefix+"/"+fname6).remove();
+
// we have to open file first.
diskAdaptor->initAndOpenFile();
SharedHandle<MultiFileAllocationIterator> itr
- (dynamic_pointer_cast<MultiFileAllocationIterator>(diskAdaptor->fileAllocationIterator()));
+ (dynamic_pointer_cast<MultiFileAllocationIterator>
+ (diskAdaptor->fileAllocationIterator()));
while(!itr->finished()) {
itr->allocateChunk();
}
- CPPUNIT_ASSERT_EQUAL((uint64_t)length1, File(dir+"/"+topDir+"/"+fname1).size());
- CPPUNIT_ASSERT_EQUAL((uint64_t)length2, File(dir+"/"+topDir+"/"+fname2).size());
- CPPUNIT_ASSERT_EQUAL((uint64_t)length3, File(dir+"/"+topDir+"/"+fname3).size());
- CPPUNIT_ASSERT(!File(dir+"/"+topDir+"/"+fname4).isFile());
+ CPPUNIT_ASSERT_EQUAL((uint64_t)length1, File(prefix+"/"+fname1).size());
+ CPPUNIT_ASSERT_EQUAL((uint64_t)length2, File(prefix+"/"+fname2).size());
+ CPPUNIT_ASSERT_EQUAL((uint64_t)length3, File(prefix+"/"+fname3).size());
+ CPPUNIT_ASSERT(!File(prefix+"/"+fname4).isFile());
- CPPUNIT_ASSERT_EQUAL((uint64_t)length5, File(dir+"/"+topDir+"/"+fname5).size());
- CPPUNIT_ASSERT(!File(dir+"/"+topDir+"/"+fname6).isFile());
+ CPPUNIT_ASSERT_EQUAL((uint64_t)length5, File(prefix+"/"+fname5).size());
+ CPPUNIT_ASSERT(!File(prefix+"/"+fname6).isFile());
} catch(Exception& e) {
CPPUNIT_FAIL(e.stackTrace());
generated by cgit (git 2.46.0) at 2026-10-12 04:42:37 +0000