From 80d361c795779d1f2c8ff9186c0b6f906d8cf3ec Mon Sep 17 00:00:00 2001
From: moneromooo-monero <moneromooo-monero@users.noreply.github.com>
Date: Wed, 16 Aug 2017 15:41:34 +0100
Subject: [PATCH] abstract_tcp_server2: improve tracking/cancelling of early
 connections

We don't actually need to keep them past the call to start, as this
adds them to the config object list, and so they'll then be cancelled
already when the stop signal arrives. This allows removing the periodic
call to cleanup connections.
---
 .../epee/include/net/abstract_tcp_server2.h   |  4 +--
 .../epee/include/net/abstract_tcp_server2.inl | 33 ++++++++-----------
 2 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/contrib/epee/include/net/abstract_tcp_server2.h b/contrib/epee/include/net/abstract_tcp_server2.h
index ca58d5467..03f143fe4 100644
--- a/contrib/epee/include/net/abstract_tcp_server2.h
+++ b/contrib/epee/include/net/abstract_tcp_server2.h
@@ -281,8 +281,6 @@ namespace net_utils
 
     bool is_thread_worker();
 
-    bool cleanup_connections();
-
     /// The io_service used to perform asynchronous operations.
     std::unique_ptr<boost::asio::io_service> m_io_service_local_instance;
     boost::asio::io_service& io_service_;    
@@ -309,7 +307,7 @@ namespace net_utils
     connection_ptr new_connection_;
 
     boost::mutex connections_mutex;
-    std::deque<std::pair<boost::system_time, connection_ptr>> connections_;
+    std::set<connection_ptr> connections_;
 
   }; // class <>boosted_tcp_server
 
diff --git a/contrib/epee/include/net/abstract_tcp_server2.inl b/contrib/epee/include/net/abstract_tcp_server2.inl
index 61276e761..76988a26e 100644
--- a/contrib/epee/include/net/abstract_tcp_server2.inl
+++ b/contrib/epee/include/net/abstract_tcp_server2.inl
@@ -54,8 +54,6 @@
 #undef MONERO_DEFAULT_LOG_CATEGORY
 #define MONERO_DEFAULT_LOG_CATEGORY "net"
 
-#define CONNECTION_CLEANUP_TIME 30 // seconds
-
 PRAGMA_WARNING_PUSH
 namespace epee
 {
@@ -808,7 +806,6 @@ POP_WARNINGS
     m_threads_count = threads_count;
     m_main_thread_id = boost::this_thread::get_id();
     MLOG_SET_THREAD_NAME("[SRV_MAIN]");
-    add_idle_handler(boost::bind(&boosted_tcp_server::cleanup_connections, this), 5000);
     while(!m_stop_signal_sent)
     {
 
@@ -898,7 +895,7 @@ POP_WARNINGS
     connections_mutex.lock();
     for (auto &c: connections_)
     {
-      c.second->cancel();
+      c->cancel();
     }
     connections_.clear();
     connections_mutex.unlock();
@@ -907,19 +904,6 @@ POP_WARNINGS
   }
   //---------------------------------------------------------------------------------
   template<class t_protocol_handler>
-  bool boosted_tcp_server<t_protocol_handler>::cleanup_connections()
-  {
-    connections_mutex.lock();
-    boost::system_time cutoff = boost::get_system_time() - boost::posix_time::seconds(CONNECTION_CLEANUP_TIME);
-    while (!connections_.empty() && connections_.front().first < cutoff)
-    {
-      connections_.pop_front();
-    }
-    connections_mutex.unlock();
-    return true;
-  }
-  //---------------------------------------------------------------------------------
-  template<class t_protocol_handler>
   bool boosted_tcp_server<t_protocol_handler>::is_stop_signal_sent()
   {
     return m_stop_signal_sent;
@@ -958,9 +942,10 @@ POP_WARNINGS
 
     connection_ptr new_connection_l(new connection<t_protocol_handler>(io_service_, m_config, m_sock_count, m_sock_number, m_pfilter, m_connection_type) );
     connections_mutex.lock();
-    connections_.push_back(std::make_pair(boost::get_system_time(), new_connection_l));
+    connections_.insert(new_connection_l);
     MDEBUG("connections_ size now " << connections_.size());
     connections_mutex.unlock();
+    epee::misc_utils::auto_scope_leave_caller scope_exit_handler = epee::misc_utils::create_scope_leave_handler([&](){ CRITICAL_REGION_LOCAL(connections_mutex); connections_.erase(new_connection_l); });
     boost::asio::ip::tcp::socket&  sock_ = new_connection_l->socket();
     
     //////////////////////////////////////////////////////////////////////////
@@ -1038,6 +1023,10 @@ POP_WARNINGS
 
     _dbg3("Connected success to " << adr << ':' << port);
 
+    // start adds the connection to the config object's list, so we don't need to have it locally anymore
+    connections_mutex.lock();
+    connections_.erase(new_connection_l);
+    connections_mutex.unlock();
     bool r = new_connection_l->start(false, 1 < m_threads_count);
     if (r)
     {
@@ -1062,9 +1051,10 @@ POP_WARNINGS
     TRY_ENTRY();    
     connection_ptr new_connection_l(new connection<t_protocol_handler>(io_service_, m_config, m_sock_count, m_sock_number, m_pfilter, m_connection_type) );
     connections_mutex.lock();
-    connections_.push_back(std::make_pair(boost::get_system_time(), new_connection_l));
+    connections_.insert(new_connection_l);
     MDEBUG("connections_ size now " << connections_.size());
     connections_mutex.unlock();
+    epee::misc_utils::auto_scope_leave_caller scope_exit_handler = epee::misc_utils::create_scope_leave_handler([&](){ CRITICAL_REGION_LOCAL(connections_mutex); connections_.erase(new_connection_l); });
     boost::asio::ip::tcp::socket&  sock_ = new_connection_l->socket();
     
     //////////////////////////////////////////////////////////////////////////
@@ -1113,6 +1103,11 @@ POP_WARNINGS
           {
             _dbg3("[sock " << new_connection_l->socket().native_handle() << "] Connected success to " << adr << ':' << port <<
               " from " << lep.address().to_string() << ':' << lep.port());
+
+            // start adds the connection to the config object's list, so we don't need to have it locally anymore
+            connections_mutex.lock();
+            connections_.erase(new_connection_l);
+            connections_mutex.unlock();
             bool r = new_connection_l->start(false, 1 < m_threads_count);
             if (r)
             {