From 6e8f51777765c34938e7b53f8985013e9a056ca8 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Mon, 6 May 2024 13:46:31 +0800 Subject: [PATCH] test/mon/test_election: free ConnectionTracker if blocked in this test, if the connection is blocked, the allocated `ConnectionTracker` is leaked. as pointed out by ASan: ``` Indirect leak of 506880 byte(s) in 10560 object(s) allocated from: #0 0x563e9d9ea1ed in operator new(unsigned long) (/home/jenkins-build/build/workspace/ceph-pull-requests/build/bin/unittest_mon_election+0x2021ed) (BuildId: 6a9fb1b76c5d1db8d2bc9957316994f90b45b6c8) #1 0x563e9da588a6 in __gnu_cxx::new_allocator > >::allocate(unsigned long, void const*) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/ext/new_allocator.h:127:27 #2 0x563e9da58830 in std::allocator > >::allocate(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/allocator.h:185:32 #3 0x563e9da58830 in std::allocator_traits > > >::allocate(std::allocator > >&, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/alloc_traits.h:464:20 #4 0x563e9da58701 in std::_Rb_tree, std::_Select1st >, std::less, std::allocator > >::_M_get_node() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_tree.h:561:16 #5 0x563e9db6f424 in std::_Rb_tree_node >* std::_Rb_tree, std::_Select1st >, std::less, std::allocator > >::_M_create_node, std::tuple<> >(std::piecewise_construct_t const&, std::tuple&&, std::tuple<>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_tree.h:611:23 #6 0x563e9db6efc0 in std::_Rb_tree_iterator > std::_Rb_tree, std::_Select1st >, std::less, std::allocator > >::_M_emplace_hint_unique, std::tuple<> >(std::_Rb_tree_const_iterator >, std::piecewise_construct_t const&, std::tuple&&, std::tuple<>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_tree.h:2431:19 #7 0x563e9db6ecb2 in std::map, std::allocator > >::operator[](int const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_map.h:501:15 #8 0x563e9db6ca32 in std::enable_if<(!(denc_traits::supported)) || (!(denc_traits::supported)), void>::type ceph::decode, std::allocator >, denc_traits, denc_traits >(std::map, std::allocator > >&, ceph::buffer::v15_2_0::list::iterator_impl&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/include/encoding.h:1095:12 #9 0x563e9db6c1d4 in ConnectionReport::decode(ceph::buffer::v15_2_0::list::iterator_impl&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ConnectionTracker.h:37:5 #10 0x563e9db6ba3c in decode(ConnectionReport&, ceph::buffer::v15_2_0::list::iterator_impl&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ConnectionTracker.h:52:1 #11 0x563e9db5a47e in std::enable_if<(!(denc_traits::supported)) || (!(denc_traits::supported)), void>::type ceph::decode, std::allocator >, denc_traits, denc_traits >(std::map, std::allocator > >&, ceph::buffer::v15_2_0::list::iterator_impl&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/include/encoding.h:1095:5 #12 0x563e9db51b69 in ConnectionTracker::decode(ceph::buffer::v15_2_0::list::iterator_impl&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ConnectionTracker.cc:309:3 #13 0x563e9da18bac in ConnectionTracker::ConnectionTracker(ceph::buffer::v15_2_0::list const&, ceph::common::CephContext*) /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ConnectionTracker.h:180:5 #14 0x563e9d9ef57f in Election::propose_to(int, int, unsigned int, ceph::buffer::v15_2_0::list&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/test_election.cc:369:15 #15 0x563e9da22ccb in Owner::propose_to_peers(unsigned int, ceph::buffer::v15_2_0::list&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/test_election.cc:145:15 #16 0x563e9db2da6c in ElectionLogic::start() /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ElectionLogic.cc:143:12 #17 0x563e9db2f128 in ElectionLogic::end_election_period() /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ElectionLogic.cc:180:7 #18 0x563e9da29a5d in Owner::election_timeout() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/test_election.cc:242:11 #19 0x563e9da19936 in Owner::notify_timestep() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/test_election.cc:282:2 #20 0x563e9d9f1181 in Election::run_timesteps(int) /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/test_election.cc:417:17 ``` in this change, we add an parameter to the handler function, so it can free the allocated `ConnectionTracker` instance. this should address the leakage reported by ASan. Signed-off-by: Kefu Chai --- src/test/mon/test_election.cc | 41 +++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/src/test/mon/test_election.cc b/src/test/mon/test_election.cc index 9dba99136e358..167d0fabf11f0 100644 --- a/src/test/mon/test_election.cc +++ b/src/test/mon/test_election.cc @@ -71,7 +71,7 @@ struct Election { void queue_timeout_message(int from, int to, function m); void queue_stable_or_timeout(int from, int to, function m, function t); - void queue_election_message(int from, int to, function m); + void queue_election_message(int from, int to, function m); // test runner interfaces int run_timesteps(int max); @@ -317,21 +317,24 @@ void Election::queue_stable_message(int from, int to, function m) } } -void Election::queue_election_message(int from, int to, function m) +void Election::queue_election_message(int from, int to, function m) { if (last_quorum_reported.count(from)) { last_quorum_change = timesteps_run; last_quorum_reported.clear(); last_leader = -1; } - if (!blocked_messages[from].count(to)) { + const bool blocked = blocked_messages[from].count(to); + if (blocked) { + return m(true); + } else { bufferlist bl; electors[from]->encode_scores(bl); Owner *o = electors[to]; messages.push_back([this,m,o,bl] { --this->pending_election_messages; o->receive_scores(bl); - m(); + m(false); }); ++pending_election_messages; } @@ -356,9 +359,11 @@ void Election::queue_stable_or_timeout(int from, int to, void Election::defer_to(int from, int to, epoch_t e) { Owner *o = electors[to]; - queue_election_message(from, to, [o, from, e] { - o->receive_ack(from, e); - }); + queue_election_message(from, to, [o, from, e](bool blocked) { + if (!blocked) { + o->receive_ack(from, e); + } + }); } void Election::propose_to(int from, int to, epoch_t e, bufferlist& cbl) @@ -366,27 +371,35 @@ void Election::propose_to(int from, int to, epoch_t e, bufferlist& cbl) Owner *o = electors[to]; ConnectionTracker *oct = NULL; if (cbl.length()) { - oct = new ConnectionTracker(cbl, g_ceph_context); // we leak these on blocked cons, meh + oct = new ConnectionTracker(cbl, g_ceph_context); } - queue_election_message(from, to, [o, from, e, oct] { - o->receive_propose(from, e, oct); + queue_election_message(from, to, [o, from, e, oct](bool blocked) { + if (blocked) { + delete oct; + } else { + o->receive_propose(from, e, oct); + } }); } void Election::claim_victory(int from, int to, epoch_t e, const set& members) { Owner *o = electors[to]; - queue_election_message(from, to, [o, from, e, members] { + queue_election_message(from, to, [o, from, e, members](bool blocked) { + if (!blocked) { o->receive_victory_claim(from, e, members); - }); + } + }); } void Election::accept_victory(int from, int to, epoch_t e) { Owner *o = electors[to]; - queue_election_message(from, to, [o, from, e] { + queue_election_message(from, to, [o, from, e](bool blocked) { + if (!blocked) { o->receive_victory_ack(from, e); - }); + } + }); } void Election::report_quorum(const set& quorum)