[C++][Parquet] Segfault when exception handling and closing RowGroupSerializer

[mapleFU]

Describe the bug, including details regarding any error messages, version, and platform.

When closing RowGroupSerializer in Parquet, the program segfault on:

CheckRowsWritten
- RowGroupSerializer::num_rows
-- RowGroupSerializer::Close
--- FileSerializer::Close
---- ~FileSerializer

And after some analyze, I found that the bug is due to exception handling. When calling FileSerializer::AppendRowGroup, the RowGroupSerializer::Close is called. So, it goes to loop:

 void Close() override {
 if (!closed_) {
 closed_ = true;
 CheckRowsWritten();

 for (size_t i = 0; i < column_writers_.size(); i++) {
 if (column_writers_[i]) {
 total_bytes_written_ += column_writers_[i]->Close();
 total_compressed_bytes_written_ +=
 column_writers_[i]->total_compressed_bytes_written();
 column_writers_[i].reset();
 }
 }

 column_writers_.clear();

 // Ensures all columns have been written
 metadata_->set_num_rows(num_rows_);
 metadata_->Finish(total_bytes_written_, row_group_ordinal_);
 }
 }

When calling column_writers_[i]->Close() in buffered mode, exception is thrown. So, assume there are 3 columns, and throw exception on the second one, column_writers_ would be:

nullptr
non-null
non-null

Thats ok, when exception propagated, it will call FileSerializer::Close:

 void Close() override {
 if (is_open_) {
 // If any functions here raise an exception, we set is_open_ to be false
 // so that this does not get called again (possibly causing segfault)
 is_open_ = false;
 if (row_group_writer_) {
 num_rows_ += row_group_writer_->num_rows();
 row_group_writer_->Close();
 }
 row_group_writer_.reset();

 WritePageIndex();

 // Write magic bytes and metadata
 auto file_encryption_properties = properties_->file_encryption_properties();

 if (file_encryption_properties == nullptr) { // Non encrypted file.
 file_metadata_ = metadata_->Finish(key_value_metadata_);
 WriteFileMetaData(*file_metadata_, sink_.get());
 } else { // Encrypted file
 CloseEncryptedFile(file_encryption_properties);
 }
 }
 }

The code looks good, however, this time, row_group_writer_ != nullptr, so row_group_writer_->num_rows() is called:

 int64_t num_rows() const override {
 CheckRowsWritten();
 // CheckRowsWritten ensures num_rows_ is set correctly
 return num_rows_;
 }

 void CheckRowsWritten() const {
 // verify when only one column is written at a time
 if (!buffered_row_group_ && column_writers_.size() > 0 && column_writers_[0]) {
 int64_t current_col_rows = column_writers_[0]->rows_written();
 if (num_rows_ == 0) {
 num_rows_ = current_col_rows;
 } else if (num_rows_ != current_col_rows) {
 ThrowRowsMisMatchError(next_column_index_, current_col_rows, num_rows_);
 }
 } else if (buffered_row_group_ &&
 column_writers_.size() > 0) { // when buffered_row_group = true
 int64_t current_col_rows = column_writers_[0]->rows_written();
 for (int i = 1; i < static_cast<int>(column_writers_.size()); i++) {
 int64_t current_col_rows_i = column_writers_[i]->rows_written();
 if (current_col_rows != current_col_rows_i) {
 ThrowRowsMisMatchError(i, current_col_rows_i, current_col_rows);
 }
 }
 num_rows_ = current_col_rows;
 }
 }

This time, it will call column_writers_[i]->..., which is a nullptr call, causing segfault.

Component(s)

C++, Parquet