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 <[email protected]>
liangtan · Jun 17, 2020 · 01969fc · 01969fc
1 parent 870e2a6
commit 01969fc
Show file tree

Hide file tree

Showing 9 changed files with 42 additions and 0 deletions.
diff --git 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
@@ -100,6 +100,8 @@ class AbstractDiskWriter : public DiskWriter {
   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
@@ -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
@@ -72,6 +72,8 @@ class AbstractSingleDiskAdaptor : public DiskAdaptor {
 
   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
@@ -114,6 +114,9 @@ class DiskAdaptor : public BinaryStream {
   // 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
@@ -85,6 +85,9 @@ class DiskWriter : public BinaryStream {
 
   // 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
@@ -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
@@ -135,6 +135,8 @@ class MultiDiskAdaptor : public DiskAdaptor {
 
   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
@@ -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();
   }