From 35b5a76faf0c97c8dce6a20499c41776cc232d84 Mon Sep 17 00:00:00 2001
From: Islam AbdelRahman <tec@fb.com>
Date: Sat, 12 Nov 2016 20:03:39 -0800
Subject: [PATCH] Fix SstFileWriter destructor

Summary:
If user did not call SstFileWriter::Finish() or called Finish() but it failed.
We need to abandon the builder, to avoid destructing it while it's open
Closes https://github.com/facebook/rocksdb/pull/1502

Differential Revision: D4171660

Pulled By: IslamAbdelRahman

fbshipit-source-id: ab6f434
---
 db/external_sst_file_test.cc | 19 +++++++++++++++++++
 table/sst_file_writer.cc     | 10 +++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/db/external_sst_file_test.cc b/db/external_sst_file_test.cc
index 04681275e..8295c2d8d 100644
--- a/db/external_sst_file_test.cc
+++ b/db/external_sst_file_test.cc
@@ -1761,6 +1761,25 @@ TEST_F(ExternalSSTFileTest, CompactionDeadlock) {
   bg_block_put.join();
 }
 
+TEST_F(ExternalSSTFileTest, DirtyExit) {
+  Options options = CurrentOptions();
+  DestroyAndReopen(options);
+  std::string file_path = sst_files_dir_ + "/dirty_exit";
+  std::unique_ptr<SstFileWriter> sst_file_writer;
+
+  // Destruct SstFileWriter without calling Finish()
+  sst_file_writer.reset(
+      new SstFileWriter(EnvOptions(), options, options.comparator));
+  ASSERT_OK(sst_file_writer->Open(file_path));
+  sst_file_writer.reset();
+
+  // Destruct SstFileWriter with a failing Finish
+  sst_file_writer.reset(
+      new SstFileWriter(EnvOptions(), options, options.comparator));
+  ASSERT_OK(sst_file_writer->Open(file_path));
+  ASSERT_NOK(sst_file_writer->Finish());
+}
+
 #endif  // ROCKSDB_LITE
 
 }  // namespace rocksdb
diff --git a/table/sst_file_writer.cc b/table/sst_file_writer.cc
index 413d9b8b4..eca52c523 100644
--- a/table/sst_file_writer.cc
+++ b/table/sst_file_writer.cc
@@ -43,7 +43,15 @@ SstFileWriter::SstFileWriter(const EnvOptions& env_options,
                              const Comparator* user_comparator)
     : rep_(new Rep(env_options, options, user_comparator)) {}
 
-SstFileWriter::~SstFileWriter() { delete rep_; }
+SstFileWriter::~SstFileWriter() {
+  if (rep_->builder) {
+    // User did not call Finish() or Finish() failed, we need to
+    // abandon the builder.
+    rep_->builder->Abandon();
+  }
+
+  delete rep_;
+}
 
 Status SstFileWriter::Open(const std::string& file_path) {
   Rep* r = rep_;