From d5ffa2532da9283abaf18d5ca4e58d07a8b2f405 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Fri, 9 Dec 2011 23:32:38 +0900
Subject: [PATCH] AbstractDiskWriter::closeFile(): Throw exception if close()
 failed.

~AbstractDiskWriter calles closeFile(), but suppresses exception.
MultiDiskAdaptor::closeFile() logs error if child
DiskWriter::closeFile() throws exception. This exception is not
rethrown. If at least one exception is caught,
MultiDiskAdaptor::closeFile() throws new DlAbortEx.
RequestGroupMan::closeFile() just logs exception and suppress each
exception. Generally, don't call closeFile() in destructor. If you
need to call it, it must suppress the exception.
---
 src/AbstractDiskWriter.cc | 14 ++++++++++++--
 src/MultiDiskAdaptor.cc   | 16 ++++++++++++++--
 src/RequestGroupMan.cc    |  6 +++++-
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/AbstractDiskWriter.cc b/src/AbstractDiskWriter.cc
index e209018a..fb6a0174 100644
--- a/src/AbstractDiskWriter.cc
+++ b/src/AbstractDiskWriter.cc
@@ -59,7 +59,10 @@ AbstractDiskWriter::AbstractDiskWriter(const std::string& filename)
 
 AbstractDiskWriter::~AbstractDiskWriter()
 {
-  closeFile();
+  try {
+    closeFile();
+  } catch(...) {
+  }
 }
 
 void AbstractDiskWriter::openFile(off_t totalLength)
@@ -78,8 +81,15 @@ void AbstractDiskWriter::openFile(off_t totalLength)
 void AbstractDiskWriter::closeFile()
 {
   if(fd_ >= 0) {
-    close(fd_);
+    int r;
+    while((r = close(fd_)) == -1 && errno == EINTR);
     fd_ = -1;
+    if(r == -1) {
+      int errNum = errno;
+      throw DL_ABORT_EX3(errNum, fmt("Failed to close file: %s",
+                                     util::safeStrerror(errNum).c_str()),
+                         error_code::FILE_IO_ERROR);
+    }
   }
 }
 
diff --git a/src/MultiDiskAdaptor.cc b/src/MultiDiskAdaptor.cc
index 17f3198d..3cb464ff 100644
--- a/src/MultiDiskAdaptor.cc
+++ b/src/MultiDiskAdaptor.cc
@@ -297,8 +297,20 @@ void MultiDiskAdaptor::openExistingFile()
 
 void MultiDiskAdaptor::closeFile()
 {
-  std::for_each(diskWriterEntries_.begin(), diskWriterEntries_.end(),
-                mem_fun_sh(&DiskWriterEntry::closeFile));
+  bool ok = true;
+  for(std::vector<SharedHandle<DiskWriterEntry> >::const_iterator i =
+        diskWriterEntries_.begin(), eoi = diskWriterEntries_.end(); i != eoi;
+      ++i) {
+    try {
+      (*i)->closeFile();
+    } catch(RecoverableException& e) {
+      A2_LOG_ERROR_EX(EX_EXCEPTION_CAUGHT, e);
+      ok = false;
+    }
+  }
+  if(!ok) {
+    throw DL_ABORT_EX("Failed to close some files");
+  }
 }
 
 namespace {
diff --git a/src/RequestGroupMan.cc b/src/RequestGroupMan.cc
index 34e9e127..6aeae42d 100644
--- a/src/RequestGroupMan.cc
+++ b/src/RequestGroupMan.cc
@@ -554,7 +554,11 @@ void RequestGroupMan::closeFile()
 {
   for(std::deque<SharedHandle<RequestGroup> >::const_iterator itr =
         requestGroups_.begin(), eoi = requestGroups_.end(); itr != eoi; ++itr) {
-    (*itr)->closeFile();
+    try {
+      (*itr)->closeFile();
+    } catch(RecoverableException& e) {
+      A2_LOG_ERROR_EX(EX_EXCEPTION_CAUGHT, e);
+    }
   }
 }