From 3379d1466f357ae4e1e15f068a35d7ea2fddb4d1 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Mon, 21 Feb 2022 18:50:50 -0800 Subject: [PATCH] Fix DBTest2.BackupFileTemperature memory leak (#9610) Summary: Valgrind was failing with the below error because we forgot to destroy the `BackupEngine` object: ``` ==421173== Command: ./db_test2 --gtest_filter=DBTest2.BackupFileTemperature ==421173== Note: Google Test filter = DBTest2.BackupFileTemperature [==========] Running 1 test from 1 test case. [----------] Global test environment set-up. [----------] 1 test from DBTest2 [ RUN ] DBTest2.BackupFileTemperature --421173-- WARNING: unhandled amd64-linux syscall: 425 --421173-- You may be able to write your own handler. --421173-- Read the file README_MISSING_SYSCALL_OR_IOCTL. --421173-- Nevertheless we consider this a bug. Please report --421173-- it at http://valgrind.org/support/bug_reports.html. [ OK ] DBTest2.BackupFileTemperature (3366 ms) [----------] 1 test from DBTest2 (3371 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test case ran. (3413 ms total) [ PASSED ] 1 test. ==421173== ==421173== HEAP SUMMARY: ==421173== in use at exit: 13,042 bytes in 195 blocks ==421173== total heap usage: 26,022 allocs, 25,827 frees, 27,555,265 bytes allocated ==421173== ==421173== 8 bytes in 1 blocks are possibly lost in loss record 6 of 167 ==421173== at 0x4838DBF: operator new(unsigned long) (vg_replace_malloc.c:344) ==421173== by 0x8D4606: allocate (new_allocator.h:114) ==421173== by 0x8D4606: allocate (alloc_traits.h:445) ==421173== by 0x8D4606: _M_allocate (stl_vector.h:343) ==421173== by 0x8D4606: reserve (vector.tcc:78) ==421173== by 0x8D4606: rocksdb::BackupEngineImpl::Initialize() (backupable_db.cc:1174) ==421173== by 0x8D5473: Initialize (backupable_db.cc:918) ==421173== by 0x8D5473: rocksdb::BackupEngine::Open(rocksdb::BackupEngineOptions const&, rocksdb::Env*, rocksdb::BackupEngine**) (backupable_db.cc:937) ==421173== by 0x50AC8F: Open (backup_engine.h:585) ==421173== by 0x50AC8F: rocksdb::DBTest2_BackupFileTemperature_Test::TestBody() (db_test2.cc:6996) ... ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/9610 Test Plan: ``` $ make -j24 ROCKSDBTESTS_SUBSET=db_test2 valgrind_check_some ``` Reviewed By: akankshamahajan15 Differential Revision: D34371210 Pulled By: ajkr fbshipit-source-id: 68154fcb0c51b28222efa23fa4ee02df8d925a18 --- db/db_test2.cc | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/db/db_test2.cc b/db/db_test2.cc index 16397e4d9..d8f54938f 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -6990,13 +6990,17 @@ TEST_F(DBTest2, BackupFileTemperature) { for (auto info : infos) { temperatures.emplace(info.file_number, info.temperature); } - BackupEngine* backup_engine; - auto backup_options = BackupEngineOptions( - dbname_ + kFilePathSeparator + "tempbk", backup_env.get()); - auto s = BackupEngine::Open(backup_env.get(), backup_options, &backup_engine); - ASSERT_OK(s); - s = backup_engine->CreateNewBackup(db_); - ASSERT_OK(s); + + std::unique_ptr backup_engine; + { + BackupEngine* backup_engine_raw_ptr; + auto backup_options = BackupEngineOptions( + dbname_ + kFilePathSeparator + "tempbk", backup_env.get()); + ASSERT_OK(BackupEngine::Open(backup_env.get(), backup_options, + &backup_engine_raw_ptr)); + backup_engine.reset(backup_engine_raw_ptr); + } + ASSERT_OK(backup_engine->CreateNewBackup(db_)); // checking src file src_temperature hints: 2 sst files: 1 sst is kWarm, // another is kUnknown