Fix an object lifetime bug in net load tests

The commands handler must not be destroyed before the config
object, or we'll be accessing freed memory.

An earlier attempt at using boost::shared_ptr to control object
lifetime turned out to be very invasive, though would be a
better solution in theory.

3 files changed, 12 insertions, 6 deletions

diff --git a/tests/net_load_tests/clt.cpp b/tests/net_load_tests/clt.cpp
index 376d7ee53..c930a5b57 100644
--- a/tests/net_load_tests/clt.cpp
+++ b/tests/net_load_tests/clt.cpp
@@ -193,7 +193,7 @@ namespace
     {
       m_thread_count = (std::max)(min_thread_count, boost::thread::hardware_concurrency() / 2);
 
-      m_tcp_server.get_config_object().m_pcommands_handler = &m_commands_handler;
+      m_tcp_server.get_config_object().set_handler(&m_commands_handler);
       m_tcp_server.get_config_object().m_invoke_timeout = CONNECTION_TIMEOUT;
 
       ASSERT_TRUE(m_tcp_server.init_server(clt_port, "127.0.0.1"));
@@ -238,9 +238,10 @@ namespace
     static void TearDownTestCase()
     {
       // Stop server
-      test_levin_commands_handler commands_handler;
-      test_tcp_server tcp_server(epee::net_utils::e_connection_type_NET);
-      tcp_server.get_config_object().m_pcommands_handler = &commands_handler;
+      test_levin_commands_handler *commands_handler_ptr = new test_levin_commands_handler();
+      test_levin_commands_handler &commands_handler = *commands_handler_ptr;
+      test_tcp_server tcp_server(epee::net_utils::e_connection_type_RPC);
+      tcp_server.get_config_object().set_handler(commands_handler_ptr, [](epee::levin::levin_commands_handler<test_connection_context> *handler)->void { delete handler; });
       tcp_server.get_config_object().m_invoke_timeout = CONNECTION_TIMEOUT;
 
       if (!tcp_server.init_server(clt_port, "127.0.0.1")) return;
diff --git a/tests/net_load_tests/net_load_tests.h b/tests/net_load_tests/net_load_tests.h
index f74282683..ce9d8b6fe 100644
--- a/tests/net_load_tests/net_load_tests.h
+++ b/tests/net_load_tests/net_load_tests.h
@@ -151,6 +151,11 @@ namespace net_load_tests
     bool handle_new_connection(const boost::uuids::uuid& connection_id, bool ignore_close_fails = false)
     {
       size_t idx = m_next_opened_conn_idx.fetch_add(1, std::memory_order_relaxed);
+      if (idx >= m_connections.size())
+      {
+        LOG_PRINT_L0("ERROR: connections overflow");
+        exit(1);
+      }
       m_connections[idx] = connection_id;
 
       size_t prev_connection_count = m_opened_connection_count.fetch_add(1, std::memory_order_relaxed);
diff --git a/tests/net_load_tests/srv.cpp b/tests/net_load_tests/srv.cpp
index e6dee1639..f0b0889ea 100644
--- a/tests/net_load_tests/srv.cpp
+++ b/tests/net_load_tests/srv.cpp
@@ -224,8 +224,8 @@ int main(int argc, char** argv)
   if (!tcp_server.init_server(srv_port, "127.0.0.1"))
     return 1;
 
-  srv_levin_commands_handler commands_handler(tcp_server);
-  tcp_server.get_config_object().m_pcommands_handler = &commands_handler;
+  srv_levin_commands_handler *commands_handler = new srv_levin_commands_handler(tcp_server);
+  tcp_server.get_config_object().set_handler(commands_handler, [](epee::levin::levin_commands_handler<test_connection_context> *handler) { delete handler; });
   tcp_server.get_config_object().m_invoke_timeout = 10000;
   //tcp_server.get_config_object().m_max_packet_size = max_packet_size;