From 870e2a6014462c55cc252140f5e215374881c6e9 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 17 Jun 2020 00:08:55 +0300 Subject: flush internal buffers before auto-saving control file Otherwise, some pieces may be marked as finished in the control file though they have not yet been written to the storage file. This should prevent data loss and corruption when resuming downloads after an app crash. Signed-off-by: Ali MJ Al-Nasrawy --- src/DefaultPieceStorage.cc | 6 ++++-- src/DefaultPieceStorage.h | 2 +- src/PieceStorage.h | 5 +++-- src/RequestGroup.cc | 5 ++++- src/UnknownLengthPieceStorage.h | 2 +- test/MockPieceStorage.h | 2 +- 6 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/DefaultPieceStorage.cc b/src/DefaultPieceStorage.cc index ceff957b..e08b1b18 100644 --- a/src/DefaultPieceStorage.cc +++ b/src/DefaultPieceStorage.cc @@ -685,7 +685,7 @@ std::shared_ptr DefaultPieceStorage::getDiskAdaptor() WrDiskCache* DefaultPieceStorage::getWrDiskCache() { return wrDiskCache_; } -void DefaultPieceStorage::flushWrDiskCacheEntry() +void DefaultPieceStorage::flushWrDiskCacheEntry(bool releaseEntries) { if (!wrDiskCache_) { return; @@ -697,7 +697,9 @@ void DefaultPieceStorage::flushWrDiskCacheEntry() auto ce = piece->getWrDiskCacheEntry(); if (ce) { piece->flushWrCache(wrDiskCache_); - piece->releaseWrCache(wrDiskCache_); + if (releaseEntries) { + piece->releaseWrCache(wrDiskCache_); + } } } } diff --git a/src/DefaultPieceStorage.h b/src/DefaultPieceStorage.h index e41736ab..4d2cdbe0 100644 --- a/src/DefaultPieceStorage.h +++ b/src/DefaultPieceStorage.h @@ -234,7 +234,7 @@ public: virtual WrDiskCache* getWrDiskCache() CXX11_OVERRIDE; - virtual void flushWrDiskCacheEntry() CXX11_OVERRIDE; + virtual void flushWrDiskCacheEntry(bool releaseEntries) CXX11_OVERRIDE; virtual int32_t getPieceLength(size_t index) CXX11_OVERRIDE; diff --git a/src/PieceStorage.h b/src/PieceStorage.h index c6ea6750..a987fb24 100644 --- a/src/PieceStorage.h +++ b/src/PieceStorage.h @@ -226,8 +226,9 @@ public: virtual WrDiskCache* getWrDiskCache() = 0; - // Flushes write disk cache for in-flight piece and evicts them. - virtual void flushWrDiskCacheEntry() = 0; + // Flushes write disk cache for in-flight piece + // and optionally releases the associated cache entries. + virtual void flushWrDiskCacheEntry(bool releaseEntries) = 0; virtual int32_t getPieceLength(size_t index) = 0; diff --git a/src/RequestGroup.cc b/src/RequestGroup.cc index c05a8189..a008642d 100644 --- a/src/RequestGroup.cc +++ b/src/RequestGroup.cc @@ -210,7 +210,7 @@ std::pair RequestGroup::downloadResult() const void RequestGroup::closeFile() { if (pieceStorage_) { - pieceStorage_->flushWrDiskCacheEntry(); + pieceStorage_->flushWrDiskCacheEntry(true); pieceStorage_->getDiskAdaptor()->closeFile(); } } @@ -1290,6 +1290,9 @@ bool RequestGroup::doesUploadSpeedExceed() void RequestGroup::saveControlFile() const { if (saveControlFile_) { + if (pieceStorage_) { + pieceStorage_->flushWrDiskCacheEntry(false); + } progressInfoFile_->save(); } } diff --git a/src/UnknownLengthPieceStorage.h b/src/UnknownLengthPieceStorage.h index 2b05918c..8c3dac89 100644 --- a/src/UnknownLengthPieceStorage.h +++ b/src/UnknownLengthPieceStorage.h @@ -216,7 +216,7 @@ public: virtual WrDiskCache* getWrDiskCache() CXX11_OVERRIDE { return nullptr; } - virtual void flushWrDiskCacheEntry() CXX11_OVERRIDE {} + virtual void flushWrDiskCacheEntry(bool releaseEntries) CXX11_OVERRIDE {} virtual int32_t getPieceLength(size_t index) CXX11_OVERRIDE; diff --git a/test/MockPieceStorage.h b/test/MockPieceStorage.h index 379f9743..a9a9f93f 100644 --- a/test/MockPieceStorage.h +++ b/test/MockPieceStorage.h @@ -228,7 +228,7 @@ public: virtual WrDiskCache* getWrDiskCache() CXX11_OVERRIDE { return 0; } - virtual void flushWrDiskCacheEntry() CXX11_OVERRIDE {} + virtual void flushWrDiskCacheEntry(bool releaseEntries) CXX11_OVERRIDE {} void setDiskAdaptor(const std::shared_ptr& adaptor) { -- cgit From 01969fc5304a738718c00b6153d672ce4f085747 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 17 Jun 2020 09:51:04 +0300 Subject: flush OS write buffers before saving control file This ensures that pieces are physically written to disk before marking them as finished in the control file. This should prevent data loss and corruption when resuming downloads after a system crash. Signed-off-by: Ali MJ Al-Nasrawy --- src/AbstractDiskWriter.cc | 12 ++++++++++++ src/AbstractDiskWriter.h | 2 ++ src/AbstractSingleDiskAdaptor.cc | 5 +++++ src/AbstractSingleDiskAdaptor.h | 2 ++ src/DiskAdaptor.h | 3 +++ src/DiskWriter.h | 3 +++ src/MultiDiskAdaptor.cc | 11 +++++++++++ src/MultiDiskAdaptor.h | 2 ++ src/RequestGroup.cc | 2 ++ 9 files changed, 42 insertions(+) diff --git a/src/AbstractDiskWriter.cc b/src/AbstractDiskWriter.cc index ff4f8856..f21c5261 100644 --- a/src/AbstractDiskWriter.cc +++ b/src/AbstractDiskWriter.cc @@ -590,4 +590,16 @@ void AbstractDiskWriter::dropCache(int64_t len, int64_t offset) #endif // HAVE_POSIX_FADVISE } +void AbstractDiskWriter::flushOSBuffers() +{ + if (fd_ == A2_BAD_FD) { + return; + } +#ifdef __MINGW32__ + FlushFileBuffers(fd_); +#else // !__MINGW32__ + fsync(fd_); +#endif // __MINGW32__ +} + } // namespace aria2 diff --git a/src/AbstractDiskWriter.h b/src/AbstractDiskWriter.h index 9e5c4799..96518a68 100644 --- a/src/AbstractDiskWriter.h +++ b/src/AbstractDiskWriter.h @@ -100,6 +100,8 @@ public: virtual void enableMmap() CXX11_OVERRIDE; virtual void dropCache(int64_t len, int64_t offset) CXX11_OVERRIDE; + + virtual void flushOSBuffers() CXX11_OVERRIDE; }; } // namespace aria2 diff --git a/src/AbstractSingleDiskAdaptor.cc b/src/AbstractSingleDiskAdaptor.cc index 54ec783e..6e6b70e6 100644 --- a/src/AbstractSingleDiskAdaptor.cc +++ b/src/AbstractSingleDiskAdaptor.cc @@ -103,6 +103,11 @@ void AbstractSingleDiskAdaptor::writeCache(const WrDiskCacheEntry* entry) } } +void AbstractSingleDiskAdaptor::flushOSBuffers() +{ + diskWriter_->flushOSBuffers(); +} + bool AbstractSingleDiskAdaptor::fileExists() { return File(getFilePath()).exists(); diff --git a/src/AbstractSingleDiskAdaptor.h b/src/AbstractSingleDiskAdaptor.h index 8d17686c..36899e10 100644 --- a/src/AbstractSingleDiskAdaptor.h +++ b/src/AbstractSingleDiskAdaptor.h @@ -72,6 +72,8 @@ public: virtual void writeCache(const WrDiskCacheEntry* entry) CXX11_OVERRIDE; + virtual void flushOSBuffers() CXX11_OVERRIDE; + virtual bool fileExists() CXX11_OVERRIDE; virtual int64_t size() CXX11_OVERRIDE; diff --git a/src/DiskAdaptor.h b/src/DiskAdaptor.h index 2643a65d..9287b3ec 100644 --- a/src/DiskAdaptor.h +++ b/src/DiskAdaptor.h @@ -114,6 +114,9 @@ public: // Writes cached data to the underlying disk. virtual void writeCache(const WrDiskCacheEntry* entry) = 0; + // Force physical write of data from OS buffer cache. + virtual void flushOSBuffers() {}; + void setFileAllocationMethod(FileAllocationMethod method) { fileAllocationMethod_ = method; diff --git a/src/DiskWriter.h b/src/DiskWriter.h index 960e346c..03a4147b 100644 --- a/src/DiskWriter.h +++ b/src/DiskWriter.h @@ -85,6 +85,9 @@ public: // Drops cache in range [offset, offset + len) virtual void dropCache(int64_t len, int64_t offset) {} + + // Force physical write of data from OS buffer cache. + virtual void flushOSBuffers() {} }; } // namespace aria2 diff --git a/src/MultiDiskAdaptor.cc b/src/MultiDiskAdaptor.cc index d4f419cf..14321aab 100644 --- a/src/MultiDiskAdaptor.cc +++ b/src/MultiDiskAdaptor.cc @@ -419,6 +419,17 @@ void MultiDiskAdaptor::writeCache(const WrDiskCacheEntry* entry) } } +void MultiDiskAdaptor::flushOSBuffers() +{ + for (auto& dwent : openedDiskWriterEntries_) { + auto& dw = dwent->getDiskWriter(); + if (!dw) { + continue; + } + dw->flushOSBuffers(); + } +} + bool MultiDiskAdaptor::fileExists() { return std::find_if(std::begin(getFileEntries()), std::end(getFileEntries()), diff --git a/src/MultiDiskAdaptor.h b/src/MultiDiskAdaptor.h index 9959888a..7f8ccef4 100644 --- a/src/MultiDiskAdaptor.h +++ b/src/MultiDiskAdaptor.h @@ -135,6 +135,8 @@ public: virtual void writeCache(const WrDiskCacheEntry* entry) CXX11_OVERRIDE; + virtual void flushOSBuffers() CXX11_OVERRIDE; + virtual bool fileExists() CXX11_OVERRIDE; virtual int64_t size() CXX11_OVERRIDE; diff --git a/src/RequestGroup.cc b/src/RequestGroup.cc index a008642d..fd6801fc 100644 --- a/src/RequestGroup.cc +++ b/src/RequestGroup.cc @@ -211,6 +211,7 @@ void RequestGroup::closeFile() { if (pieceStorage_) { pieceStorage_->flushWrDiskCacheEntry(true); + pieceStorage_->getDiskAdaptor()->flushOSBuffers(); pieceStorage_->getDiskAdaptor()->closeFile(); } } @@ -1292,6 +1293,7 @@ void RequestGroup::saveControlFile() const if (saveControlFile_) { if (pieceStorage_) { pieceStorage_->flushWrDiskCacheEntry(false); + pieceStorage_->getDiskAdaptor()->flushOSBuffers(); } progressInfoFile_->save(); } -- cgit From 2d0171e95636eead0c7d26d8bef99f451ab3e20d Mon Sep 17 00:00:00 2001 From: Natanael Copa Date: Fri, 7 Aug 2020 15:59:49 +0200 Subject: Fix segfault when time_t is 64bit on 32bit arch (#1666) On some platforms, like with musl libc, time_t may be 64 bit even on 32bit platforms. Fix segfault by convert time_t to 64 bit and use 64bit format modifier instead of assume time_t is %ld --- src/ServerStat.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ServerStat.cc b/src/ServerStat.cc index d35407dc..eb2e17ee 100644 --- a/src/ServerStat.cc +++ b/src/ServerStat.cc @@ -183,11 +183,12 @@ bool ServerStat::operator==(const ServerStat& serverStat) const std::string ServerStat::toString() const { return fmt("host=%s, protocol=%s, dl_speed=%d, sc_avg_speed=%d," - " mc_avg_speed=%d, last_updated=%ld, counter=%d, status=%s", + " mc_avg_speed=%d, last_updated=%" PRId64 + ", counter=%d, status=%s", getHostname().c_str(), getProtocol().c_str(), getDownloadSpeed(), getSingleConnectionAvgSpeed(), getMultiConnectionAvgSpeed(), - getLastUpdated().getTimeFromEpoch(), getCounter(), - STATUS_STRING[getStatus()]); + static_cast(getLastUpdated().getTimeFromEpoch()), + getCounter(), STATUS_STRING[getStatus()]); } } // namespace aria2 -- cgit From faa6955c8d7bb7b6541e0b7fbb84f7d213f761ab Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 28 Dec 2020 11:04:37 +0900 Subject: Add GitHub Actions workflow build --- .github/workflows/build.yml | 66 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 .github/workflows/build.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 00000000..58b7b4e2 --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,66 @@ +name: build + +on: push + +jobs: + build: + runs-on: ${{ matrix.os }} + + strategy: + matrix: + os: [ubuntu-20.04, macos-10.15] + compiler: [gcc, clang] + + steps: + - uses: actions/checkout@v2 + - name: Linux setup + if: runner.os == 'Linux' + run: | + sudo apt-get install \ + g++-8 \ + autoconf \ + automake \ + autotools-dev \ + autopoint \ + libtool \ + pkg-config \ + libssl-dev \ + libc-ares-dev \ + zlib1g-dev \ + libsqlite3-dev \ + libssh2-1-dev \ + libcppunit-dev + - name: MacOS setup + if: runner.os == 'macOS' + run: | + brew install cppunit gettext openssl libssh2 c-ares sqlite3 \ + autoconf automake pkg-config libtool + - name: Setup clang + if: matrix.compiler == 'clang' + run: | + echo 'CC=clang' >> $GITHUB_ENV + echo 'CXX=clang++' >> $GITHUB_ENV + - name: Setup gcc + if: runner.os == 'Linux' && matrix.compiler == 'gcc' + run: | + echo 'CC=gcc-8' >> $GITHUB_ENV + echo 'CXX=g++-8' >> $GITHUB_ENV + - name: Libtool + run: | + autoreconf -i + - name: Configure autotools (Linux) + if: runner.os == 'Linux' + run: | + ./configure \ + CPPFLAGS="-fsanitize=address" LDFLAGS="-fsanitize=address" + - name: Configure autotools (macOS) + if: runner.os == 'macOS' + run: | + ./configure \ + --without-openssl --without-gnutls --with-appletls \ + --disable-nls \ + CPPFLAGS="-fsanitize=address" LDFLAGS="-fsanitize=address" + - name: Build aria2 + run: | + make + make check -- cgit