Skip to content

Commit

Permalink
test/mon/test_election: free ConnectionTracker if blocked
Browse files Browse the repository at this point in the history 
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<std::_Rb_tree_node<std::pair<int const, double> > >::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<std::_Rb_tree_node<std::pair<int const, double> > >::allocate(unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/allocator.h:185:32
    ceph#3 0x563e9da58830 in std::allocator_traits<std::allocator<std::_Rb_tree_node<std::pair<int const, double> > > >::allocate(std::allocator<std::_Rb_tree_node<std::pair<int const, double> > >&, unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/alloc_traits.h:464:20
    ceph#4 0x563e9da58701 in std::_Rb_tree<int, std::pair<int const, double>, std::_Select1st<std::pair<int const, double> >, std::less<int>, std::allocator<std::pair<int const, double> > >::_M_get_node() /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_tree.h:561:16
    ceph#5 0x563e9db6f424 in std::_Rb_tree_node<std::pair<int const, double> >* std::_Rb_tree<int, std::pair<int const, double>, std::_Select1st<std::pair<int const, double> >, std::less<int>, std::allocator<std::pair<int const, double> > >::_M_create_node<std::piecewise_construct_t const&, std::tuple<int const&>, std::tuple<> >(std::piecewise_construct_t const&, std::tuple<int const&>&&, std::tuple<>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_tree.h:611:23
    ceph#6 0x563e9db6efc0 in std::_Rb_tree_iterator<std::pair<int const, double> > std::_Rb_tree<int, std::pair<int const, double>, std::_Select1st<std::pair<int const, double> >, std::less<int>, std::allocator<std::pair<int const, double> > >::_M_emplace_hint_unique<std::piecewise_construct_t const&, std::tuple<int const&>, std::tuple<> >(std::_Rb_tree_const_iterator<std::pair<int const, double> >, std::piecewise_construct_t const&, std::tuple<int const&>&&, std::tuple<>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_tree.h:2431:19
    ceph#7 0x563e9db6ecb2 in std::map<int, double, std::less<int>, std::allocator<std::pair<int const, double> > >::operator[](int const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_map.h:501:15
    ceph#8 0x563e9db6ca32 in std::enable_if<(!(denc_traits<int, void>::supported)) || (!(denc_traits<double, void>::supported)), void>::type ceph::decode<int, double, std::less<int>, std::allocator<std::pair<int const, double> >, denc_traits<int, void>, denc_traits<double, void> >(std::map<int, double, std::less<int>, std::allocator<std::pair<int const, double> > >&, ceph::buffer::v15_2_0::list::iterator_impl<true>&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/include/encoding.h:1095:12
    ceph#9 0x563e9db6c1d4 in ConnectionReport::decode(ceph::buffer::v15_2_0::list::iterator_impl<true>&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ConnectionTracker.h:37:5
    ceph#10 0x563e9db6ba3c in decode(ConnectionReport&, ceph::buffer::v15_2_0::list::iterator_impl<true>&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ConnectionTracker.h:52:1
    ceph#11 0x563e9db5a47e in std::enable_if<(!(denc_traits<int, void>::supported)) || (!(denc_traits<ConnectionReport, void>::supported)), void>::type ceph::decode<int, ConnectionReport, std::less<int>, std::allocator<std::pair<int const, ConnectionReport> >, denc_traits<int, void>, denc_traits<ConnectionReport, void> >(std::map<int, ConnectionReport, std::less<int>, std::allocator<std::pair<int const, ConnectionReport> > >&, ceph::buffer::v15_2_0::list::iterator_impl<true>&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/include/encoding.h:1095:5
    ceph#12 0x563e9db51b69 in ConnectionTracker::decode(ceph::buffer::v15_2_0::list::iterator_impl<true>&) /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ConnectionTracker.cc:309:3
    ceph#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
    ceph#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
    ceph#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
    ceph#16 0x563e9db2da6c in ElectionLogic::start() /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ElectionLogic.cc:143:12
    ceph#17 0x563e9db2f128 in ElectionLogic::end_election_period() /home/jenkins-build/build/workspace/ceph-pull-requests/src/mon/ElectionLogic.cc:180:7
    ceph#18 0x563e9da29a5d in Owner::election_timeout() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/test_election.cc:242:11
    ceph#19 0x563e9da19936 in Owner::notify_timestep() /home/jenkins-build/build/workspace/ceph-pull-requests/src/test/mon/test_election.cc:282:2
    ceph#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 <[email protected]>
  • Loading branch information
@tchaikov
tchaikov committed May 6, 2024
1 parent 1602143 commit 6e8f517
Showing 1 changed file with 27 additions and 14 deletions.
41 changes: 27 additions & 14 deletions src/test/mon/test_election.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ struct Election {
void queue_timeout_message(int from, int to, function<void()> m);
void queue_stable_or_timeout(int from, int to,
function<void()> m, function<void()> t);
void queue_election_message(int from, int to, function<void()> m);
void queue_election_message(int from, int to, function<void(bool)> m);

// test runner interfaces
int run_timesteps(int max);
Expand Down Expand Up @@ -317,21 +317,24 @@ void Election::queue_stable_message(int from, int to, function<void()> m)
}
}

void Election::queue_election_message(int from, int to, function<void()> m)
void Election::queue_election_message(int from, int to, function<void(bool)> 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;
}
Expand All @@ -356,37 +359,47 @@ 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)
{
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<int>& 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<int>& quorum)
Expand Down

0 comments on commit 6e8f517

Please sign in to comment.