Avoid some unhandled exceptions in destructor

Tom-Newton · Feb 18, 2024 · 67a5e20 · 67a5e20
1 parent e0d5d2b
commit 67a5e20
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 5 deletions.
diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc
@@ -730,9 +730,15 @@ class ObjectAppendStream final : public io::OutputStream {
   }
 
   ~ObjectAppendStream() override {
-    // For compliance with the rest of the IO stack, Close rather than Abort,
-    // even though it may be more expensive.
-    io::internal::CloseFromDestructor(this);
+    if (at_least_one_successful_append_) {
+      // For compliance with the rest of the IO stack, Close rather than Abort,
+      // even though it may be more expensive.
+      io::internal::CloseFromDestructor(this);
+    } else {
+      // Avoid Flushing if we don't need to. If Flush throws an exception in this 
+      // destructor it can't be handled by the caller.
+      io::internal::AbortFromDestructor(this);
+    }
   }
 
   Status Init() {
@@ -856,6 +862,7 @@ class ObjectAppendStream final : public io::OutputStream {
     block_ids_.push_back(new_block_id);
     pos_ += nbytes;
     content_length_ += nbytes;
+    at_least_one_successful_append_ = true;
     return Status::OK();
   }
 
@@ -864,6 +871,7 @@ class ObjectAppendStream final : public io::OutputStream {
   const AzureLocation location_;
 
   bool closed_ = false;
+  bool at_least_one_successful_append_ = false;
   int64_t pos_ = 0;
   int64_t content_length_ = kNoSize;
   std::vector<std::string> block_ids_;

diff --git a/cpp/src/arrow/io/interfaces.cc b/cpp/src/arrow/io/interfaces.cc
@@ -280,8 +280,7 @@ Result<std::shared_ptr<InputStream>> RandomAccessFile::GetStream(
 
 namespace internal {
 
-void CloseFromDestructor(FileInterface* file) {
-  Status st = file->Close();
+void HandleFileStatusFromDestructor(Status st, FileInterface* file) {
   if (!st.ok()) {
     auto file_type = typeid(*file).name();
 #ifdef NDEBUG
@@ -295,6 +294,16 @@ void CloseFromDestructor(FileInterface* file) {
   }
 }
 
+void CloseFromDestructor(FileInterface* file) {
+  Status st = file->Close();
+  HandleFileStatusFromDestructor(st, file);
+}
+
+void AbortFromDestructor(FileInterface* file) {
+  Status st = file->Abort();
+  HandleFileStatusFromDestructor(st, file);
+}
+
 Result<int64_t> ValidateReadRange(int64_t offset, int64_t size, int64_t file_size) {
   if (offset < 0 || size < 0) {
     return Status::Invalid("Invalid read (offset = ", offset, ", size = ", size, ")");

diff --git a/cpp/src/arrow/io/util_internal.h b/cpp/src/arrow/io/util_internal.h
@@ -31,6 +31,7 @@ namespace io {
 namespace internal {
 
 ARROW_EXPORT void CloseFromDestructor(FileInterface* file);
+ARROW_EXPORT void AbortFromDestructor(FileInterface* file);
 
 // Validate a (offset, size) region (as given to ReadAt) against
 // the file size.  Return the actual read size.