Skip to content

Commit

Permalink
Avoid errors in destructor more neatly
Browse files Browse the repository at this point in the history
  • Loading branch information
Tom-Newton committed Feb 18, 2024
1 parent 37862c2 commit 8b1ce80
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 22 deletions.
21 changes: 10 additions & 11 deletions cpp/src/arrow/filesystem/azurefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -738,15 +738,9 @@ class ObjectAppendStream final : public io::OutputStream {
}

~ObjectAppendStream() override {
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);
}
// For compliance with the rest of the IO stack, Close rather than Abort,
// even though it may be more expensive.
io::internal::CloseFromDestructor(this);
}

Status Init() {
Expand Down Expand Up @@ -789,6 +783,7 @@ class ObjectAppendStream final : public io::OutputStream {
block_ids_.push_back(block.Name);
}
}
initialised_ = true;
return Status::OK();
}

Expand Down Expand Up @@ -835,6 +830,11 @@ class ObjectAppendStream final : public io::OutputStream {

Status Flush() override {
RETURN_NOT_OK(CheckClosed("flush"));
if (!initialised_) {
// If the stream has not been successfully initialized then there is nothing to
// flush. This also avoids some unhandled errors when flushing in the destructor.
return Status::OK();
}
return CommitBlockList(block_blob_client_, block_ids_, metadata_);
}

Expand Down Expand Up @@ -880,7 +880,6 @@ 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();
}

Expand All @@ -892,7 +891,7 @@ class ObjectAppendStream final : public io::OutputStream {
int64_t content_length_ = kNoSize;

bool closed_ = false;
bool at_least_one_successful_append_ = false;
bool initialised_ = false;
int64_t pos_ = 0;
std::vector<std::string> block_ids_;
Storage::Metadata metadata_;
Expand Down
13 changes: 2 additions & 11 deletions cpp/src/arrow/io/interfaces.cc
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ Result<std::shared_ptr<InputStream>> RandomAccessFile::GetStream(

namespace internal {

void HandleFileStatusFromDestructor(Status st, FileInterface* file) {
void CloseFromDestructor(FileInterface* file) {
Status st = file->Close();
if (!st.ok()) {
auto file_type = typeid(*file).name();
#ifdef NDEBUG
Expand All @@ -294,16 +295,6 @@ void HandleFileStatusFromDestructor(Status st, 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, ")");
Expand Down

0 comments on commit 8b1ce80

Please sign in to comment.