From f24ef5d6ab8bcde242508536ff504f2c3e304600 Mon Sep 17 00:00:00 2001 From: Changyu Bi Date: Sat, 31 Dec 2022 10:56:55 -0800 Subject: [PATCH] Fix BackupEngineTest.ExcludeFiles memory leak (#11066) Summary: Valgrind was complaining about the test BackupEngineTest.ExcludeFiles. The cause is backup_engine not being freed similar to https://github.com/facebook/rocksdb/issues/9610. ``` ==18228== Command: ./backup_engine_test --gtest_filter=BackupEngineTest.ExcludeFiles ==18228== Note: Google Test filter = BackupEngineTest.ExcludeFiles [==========] Running 1 test from 1 test case. [----------] Global test environment set-up. [----------] 1 test from BackupEngineTest [ RUN ] BackupEngineTest.ExcludeFiles [ OK ] BackupEngineTest.ExcludeFiles (16264 ms) [----------] 1 test from BackupEngineTest (16273 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test case ran. (16306 ms total) [ PASSED ] 1 test. ==18228== ==18228== HEAP SUMMARY: ==18228== in use at exit: 14,099 bytes in 159 blocks ==18228== total heap usage: 255,328 allocs, 255,169 frees, 497,538,546 bytes allocated ==18228== ==18228== 19 bytes in 1 blocks are possibly lost in loss record 4 of 67 ==18228== at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==18228== by 0x1E752D: void std::__cxx11::basic_string, std::allocator >::_M_construct(char*, char*, std::forward_iterator_tag) [clone .constprop.0] (basic_string.tcc:219) ==18228== by 0x1F1898: _M_construct_aux (basic_string.h:251) ==18228== by 0x1F1898: _M_construct (basic_string.h:270) ==18228== by 0x1F1898: basic_string (basic_string.h:455) ==18228== by 0x1F1898: construct, const std::__cxx11::basic_string, std::allocator >&> (new_allocator.h:146) ==18228== by 0x1F1898: construct, const std::__cxx11::basic_string, std::allocator >&> (alloc_traits.h:483) ==18228== by 0x1F1898: push_back (stl_vector.h:1189) ==18228== by 0x1F1898: rocksdb::(anonymous namespace)::TestFs::NewWritableFile(std::__cxx11::basic_string, std::allocator > const&, rocksdb::FileOptions const&, std::unique_ptr >*, rocksdb::IODebugContext*) (backup_engine_test.cc:208) ==18228== by 0x4B3583: rocksdb::NewWritableFile(rocksdb::FileSystem*, std::__cxx11::basic_string, std::allocator > const&, std::unique_ptr >*, rocksdb::FileOptions const&) (read_write_util.cc:23) ==18228== by 0x31C3A8: rocksdb::DBImpl::CreateWAL(unsigned long, unsigned long, unsigned long, rocksdb::log::Writer**) (db_impl_open.cc:1752) ==18228== by 0x321A8C: rocksdb::DBImpl::Open(rocksdb::DBOptions const&, std::__cxx11::basic_string, std::allocator > const&, std::vector > const&, std::vector >*, rocksdb::DB**, bool, bool) (db_impl_open.cc:1852) ==18228== by 0x322E7F: Open (db_impl_open.cc:1660) ==18228== by 0x322E7F: rocksdb::DB::Open(rocksdb::Options const&, std::__cxx11::basic_string, std::allocator > const&, rocksdb::DB**) (db_impl_open.cc:1637) ==18228== by 0x1EE1CD: InitializeDBAndBackupEngine (backup_engine_test.cc:724) ==18228== by 0x1EE1CD: rocksdb::(anonymous namespace)::BackupEngineTest::OpenDBAndBackupEngine(bool, bool, rocksdb::(anonymous namespace)::BackupEngineTest::ShareOption) (backup_engine_test.cc:732) ==18228== by 0x217585: rocksdb::(anonymous namespace)::BackupEngineTest_ExcludeFiles_Test::TestBody() (backup_engine_test.cc:4232) ==18228== by 0x296143: HandleSehExceptionsInMethodIfSupported (gtest-all.cc:3899) ==18228== by 0x296143: void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (gtest-all.cc:3935) ==18228== by 0x28A0A5: testing::Test::Run() [clone .part.0] (gtest-all.cc:3973) ==18228== by 0x28A364: Run (gtest-all.cc:3965) ==18228== by 0x28A364: testing::TestInfo::Run() [clone .part.0] (gtest-all.cc:4149) ... ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/11066 Test Plan: make -j24 J=24 ROCKSDBTESTS_SUBSET=backup_engine_test valgrind_check_some Reviewed By: ajkr Differential Revision: D42297791 Pulled By: cbi42 fbshipit-source-id: db67982b27b91cc78e1a9f4a96da0cba7c9785b7 --- utilities/backup/backup_engine_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/utilities/backup/backup_engine_test.cc b/utilities/backup/backup_engine_test.cc index 0de904896..d780a1b2b 100644 --- a/utilities/backup/backup_engine_test.cc +++ b/utilities/backup/backup_engine_test.cc @@ -4389,6 +4389,8 @@ TEST_F(BackupEngineTest, ExcludeFiles) { // Includes check for leaked backups files AssertBackupInfoConsistency(/*allow excluded*/ false); + + delete alt_backup_engine; } } // namespace