]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
test/mon/test_election: free ConnectionTracker if blocked 57277/head
authorKefu Chai <tchaikov@gmail.com>
Mon, 6 May 2024 05:46:31 +0000 (13:46 +0800)
committerKefu Chai <tchaikov@gmail.com>
Mon, 6 May 2024 05:51:31 +0000 (13:51 +0800)
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
    #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
    #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
    #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
    #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
    #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
    #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
    #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
    #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
    #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
    #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
    #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 <tchaikov@gmail.com>
src/test/mon/test_election.cc

index 9dba99136e358c1e5cb9754bbe36db4cdb1a573c..167d0fabf11f01068c4cfd27f0b30d65da0b7b32 100644 (file)
@@ -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);
@@ -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;
   }
@@ -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<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)