Fix memory leak on error in C API create_column_family (facebook#14447)

Summary:
Pull Request resolved: facebook#14447

`rocksdb_create_column_family()` and
`rocksdb_transactiondb_create_column_family()` allocate a
`rocksdb_column_family_handle_t` but always return it even when
`CreateColumnFamily()` fails. This leaks the handle and returns an object
with an indeterminate `rep` pointer. Other similar functions like
`rocksdb_create_column_family_with_import()` correctly delete the handle
and return nullptr on error.

Fix: Initialize `handle->rep = nullptr`, check `SaveError()` return value,
and on error delete the handle and return nullptr.

Reviewed By: anand1976

Differential Revision: D95303444

fbshipit-source-id: 5fde7a3ed588794d78429d5cb3d9f621f0fb6388

Author: omkarhgawde

db/c.cc

@@ -1759,9 +1759,13 @@ rocksdb_column_family_handle_t* rocksdb_create_column_family(
     rocksdb_t* db, const rocksdb_options_t* column_family_options,
     const char* column_family_name, char** errptr) {
   rocksdb_column_family_handle_t* handle = new rocksdb_column_family_handle_t;
-  SaveError(errptr, db->rep->CreateColumnFamily(
-                        ColumnFamilyOptions(column_family_options->rep),
-                        std::string(column_family_name), &(handle->rep)));
+  handle->rep = nullptr;
+  if (SaveError(errptr, db->rep->CreateColumnFamily(
+                            ColumnFamilyOptions(column_family_options->rep),
+                            std::string(column_family_name), &(handle->rep)))) {
+    delete handle;
+    return nullptr;
+  }
   handle->immortal = false;
   return handle;
 }
@@ -7534,9 +7538,13 @@ rocksdb_column_family_handle_t* rocksdb_transactiondb_create_column_family(
     const rocksdb_options_t* column_family_options,
     const char* column_family_name, char** errptr) {
   rocksdb_column_family_handle_t* handle = new rocksdb_column_family_handle_t;
-  SaveError(errptr, txn_db->rep->CreateColumnFamily(
-                        ColumnFamilyOptions(column_family_options->rep),
-                        std::string(column_family_name), &(handle->rep)));
+  handle->rep = nullptr;
+  if (SaveError(errptr, txn_db->rep->CreateColumnFamily(
+                            ColumnFamilyOptions(column_family_options->rep),
+                            std::string(column_family_name), &(handle->rep)))) {
+    delete handle;
+    return nullptr;
+  }
   handle->immortal = false;
   return handle;
 }

db/c_test.c

@@ -4880,6 +4880,21 @@ int main(int argc, char** argv) {
     rocksdb_sst_file_manager_destroy(sst_file_manager);
   }
 
+  StartPhase("create_column_family_error_returns_null");
+  {
+    // Creating a column family with a name that already exists should fail
+    // and return NULL. Without the fix, the handle is leaked and a non-NULL
+    // pointer with an indeterminate rep field is returned.
+    char* cf_err = NULL;
+    rocksdb_column_family_handle_t* cf_handle =
+        rocksdb_create_column_family(db, options, "default", &cf_err);
+    // Should have an error since "default" already exists
+    CheckCondition(cf_err != NULL);
+    // The handle should be NULL on error (this is the bug fix)
+    CheckCondition(cf_handle == NULL);
+    free(cf_err);
+  }
+
   StartPhase("cancel_all_background_work");
   rocksdb_cancel_all_background_work(db, 1);