Skip to content

Commit e43171d

Browse files
omkarhgawdemeta-codesync[bot]
authored andcommitted
Fix memory leak in ExportColumnFamily for empty column families (facebook#14458)
Summary: Pull Request resolved: facebook#14458 In `CheckpointImpl::ExportColumnFamily()`, the assignment `*metadata = result_metadata` is inside the `for` loop over `db_metadata.levels`. If the column family has no levels, the loop body never executes, so the `new ExportImportFilesMetaData()` is leaked and the caller receives a nullptr despite a success status. Fix: Move `*metadata = result_metadata` outside the loop. Reviewed By: anand1976 Differential Revision: D95303457 fbshipit-source-id: 6a9be47bcca257803969eb3daac7b91e95143ebf
1 parent 1e1097d commit e43171d

2 files changed

Lines changed: 23 additions & 1 deletion

File tree

utilities/checkpoint/checkpoint_impl.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,8 +419,8 @@ Status CheckpointImpl::ExportColumnFamily(
419419
live_file_metadata.largest = file_metadata.largest;
420420
result_metadata->files.push_back(live_file_metadata);
421421
}
422-
*metadata = result_metadata;
423422
}
423+
*metadata = result_metadata;
424424
ROCKS_LOG_INFO(db_options.info_log, "[%s] Export succeeded.",
425425
cf_name.c_str());
426426
} else {

utilities/checkpoint/checkpoint_test.cc

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,28 @@ TEST_F(CheckpointTest, ExportColumnFamilyWithLinks) {
434434
}
435435
}
436436

437+
TEST_F(CheckpointTest, ExportEmptyColumnFamily) {
438+
// Verify that exporting a column family with no levels (empty CF) does not
439+
// leak the allocated ExportImportFilesMetaData and correctly sets *metadata.
440+
auto options = CurrentOptions();
441+
options.create_if_missing = true;
442+
CreateAndReopenWithCF({}, options);
443+
444+
// Do NOT put any data — the default CF has no levels.
445+
446+
Checkpoint* checkpoint;
447+
ASSERT_OK(Checkpoint::Create(db_.get(), &checkpoint));
448+
449+
ASSERT_OK(checkpoint->ExportColumnFamily(db_->DefaultColumnFamily(),
450+
export_path_, &metadata_));
451+
// metadata_ must be set even when the CF has no files.
452+
ASSERT_NE(metadata_, nullptr);
453+
ASSERT_EQ(metadata_->files.size(), 0);
454+
ASSERT_EQ(metadata_->db_comparator_name, options.comparator->Name());
455+
456+
delete checkpoint;
457+
}
458+
437459
TEST_F(CheckpointTest, ExportColumnFamilyNegativeTest) {
438460
// Create a database
439461
auto options = CurrentOptions();

0 commit comments

Comments
 (0)