From 1de1c79d4f72a1c7aa3ad51e6a582df5c8302c39 Mon Sep 17 00:00:00 2001 From: Simon Vetter Date: Sun, 31 Oct 2021 14:51:41 +0100 Subject: [PATCH] libi2pd: add missing locks to i2p::tunnel::Tunnels m_InboundTunnelsMutex, m_OutboundTunnelsMutex and m_PoolsMutex have been changed to recursive_mutexes since they can be acquired multiple times by the same thread. --- libi2pd/Tunnel.cpp | 33 ++++++++++++++++++++++++++++----- libi2pd/Tunnel.h | 8 +++++++- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/libi2pd/Tunnel.cpp b/libi2pd/Tunnel.cpp index a315ff49..f3cdd432 100644 --- a/libi2pd/Tunnel.cpp +++ b/libi2pd/Tunnel.cpp @@ -366,6 +366,7 @@ namespace tunnel std::shared_ptr Tunnels::GetTunnel (uint32_t tunnelID) { + std::unique_lock l(m_TunnelsMutex); auto it = m_Tunnels.find(tunnelID); if (it != m_Tunnels.end ()) return it->second; @@ -374,11 +375,13 @@ namespace tunnel std::shared_ptr Tunnels::GetPendingInboundTunnel (uint32_t replyMsgID) { + std::unique_lock l(m_PendingInboundTunnelsMutex); return GetPendingTunnel (replyMsgID, m_PendingInboundTunnels); } std::shared_ptr Tunnels::GetPendingOutboundTunnel (uint32_t replyMsgID) { + std::unique_lock l(m_PendingOutboundTunnelsMutex); return GetPendingTunnel (replyMsgID, m_PendingOutboundTunnels); } @@ -459,9 +462,11 @@ namespace tunnel void Tunnels::AddTransitTunnel (std::shared_ptr tunnel) { - if (m_Tunnels.emplace (tunnel->GetTunnelID (), tunnel).second) + std::unique_lock l(m_TunnelsMutex); + if (m_Tunnels.emplace (tunnel->GetTunnelID (), tunnel).second) { + std::unique_lock l(m_TransitTunnelsMutex); m_TransitTunnels.push_back (tunnel); - else + } else LogPrint (eLogError, "Tunnel: tunnel with id ", tunnel->GetTunnelID (), " already exists"); } @@ -616,7 +621,10 @@ namespace tunnel void Tunnels::ManagePendingTunnels () { + std::unique_lock li(m_PendingInboundTunnelsMutex); ManagePendingTunnels (m_PendingInboundTunnels); + li.unlock(); + std::unique_lock lo(m_PendingOutboundTunnelsMutex); ManagePendingTunnels (m_PendingOutboundTunnels); } @@ -676,6 +684,7 @@ namespace tunnel void Tunnels::ManageOutboundTunnels () { + std::unique_lock l(m_OutboundTunnelsMutex); uint64_t ts = i2p::util::GetSecondsSinceEpoch (); { for (auto it = m_OutboundTunnels.begin (); it != m_OutboundTunnels.end ();) @@ -730,6 +739,8 @@ namespace tunnel void Tunnels::ManageInboundTunnels () { + std::unique_lock lt(m_TunnelsMutex); + std::unique_lock li(m_InboundTunnelsMutex); uint64_t ts = i2p::util::GetSecondsSinceEpoch (); { for (auto it = m_InboundTunnels.begin (); it != m_InboundTunnels.end ();) @@ -786,6 +797,7 @@ namespace tunnel return; } + std::unique_lock lo(m_OutboundTunnelsMutex); if (m_OutboundTunnels.empty () || m_InboundTunnels.size () < 3) { // trying to create one more inbound tunnel @@ -806,6 +818,8 @@ namespace tunnel void Tunnels::ManageTransitTunnels () { + std::unique_lock lt(m_TunnelsMutex); + std::unique_lock l(m_TransitTunnelsMutex); uint32_t ts = i2p::util::GetSecondsSinceEpoch (); for (auto it = m_TransitTunnels.begin (); it != m_TransitTunnels.end ();) { @@ -876,17 +890,20 @@ namespace tunnel void Tunnels::AddPendingTunnel (uint32_t replyMsgID, std::shared_ptr tunnel) { + std::unique_lock l(m_PendingInboundTunnelsMutex); m_PendingInboundTunnels[replyMsgID] = tunnel; } void Tunnels::AddPendingTunnel (uint32_t replyMsgID, std::shared_ptr tunnel) { + std::unique_lock l(m_PendingOutboundTunnelsMutex); m_PendingOutboundTunnels[replyMsgID] = tunnel; } void Tunnels::AddOutboundTunnel (std::shared_ptr newTunnel) { // we don't need to insert it to m_Tunnels + std::unique_lock l(m_OutboundTunnelsMutex); m_OutboundTunnels.push_back (newTunnel); auto pool = newTunnel->GetTunnelPool (); if (pool && pool->IsActive ()) @@ -897,8 +914,10 @@ namespace tunnel void Tunnels::AddInboundTunnel (std::shared_ptr newTunnel) { + std::unique_lock l(m_TunnelsMutex); if (m_Tunnels.emplace (newTunnel->GetTunnelID (), newTunnel).second) { + std::unique_lock l(m_InboundTunnelsMutex); m_InboundTunnels.push_back (newTunnel); auto pool = newTunnel->GetTunnelPool (); if (!pool) @@ -926,6 +945,8 @@ namespace tunnel auto inboundTunnel = std::make_shared (); inboundTunnel->SetTunnelPool (pool); inboundTunnel->SetState (eTunnelStateEstablished); + std::unique_lock lt(m_TunnelsMutex); + std::unique_lock li(m_InboundTunnelsMutex); m_InboundTunnels.push_back (inboundTunnel); m_Tunnels[inboundTunnel->GetTunnelID ()] = inboundTunnel; return inboundTunnel; @@ -936,6 +957,7 @@ namespace tunnel auto outboundTunnel = std::make_shared (); outboundTunnel->SetTunnelPool (pool); outboundTunnel->SetState (eTunnelStateEstablished); + std::unique_lock l(m_OutboundTunnelsMutex); m_OutboundTunnels.push_back (outboundTunnel); // we don't insert into m_Tunnels return outboundTunnel; @@ -964,6 +986,7 @@ namespace tunnel int timeout = 0; uint32_t ts = i2p::util::GetSecondsSinceEpoch (); // TODO: possible race condition with I2PControl + std::unique_lock l(m_TransitTunnelsMutex); for (const auto& it : m_TransitTunnels) { int t = it->GetCreationTime () + TUNNEL_EXPIRATION_TIMEOUT - ts; @@ -974,19 +997,19 @@ namespace tunnel size_t Tunnels::CountTransitTunnels() const { - // TODO: locking + std::unique_lock l(m_TransitTunnelsMutex); return m_TransitTunnels.size(); } size_t Tunnels::CountInboundTunnels() const { - // TODO: locking + std::unique_lock l(m_InboundTunnelsMutex); return m_InboundTunnels.size(); } size_t Tunnels::CountOutboundTunnels() const { - // TODO: locking + std::unique_lock l(m_OutboundTunnelsMutex); return m_OutboundTunnels.size(); } } diff --git a/libi2pd/Tunnel.h b/libi2pd/Tunnel.h index 80d29a15..640a9ca7 100644 --- a/libi2pd/Tunnel.h +++ b/libi2pd/Tunnel.h @@ -254,12 +254,18 @@ namespace tunnel bool m_IsRunning; std::thread * m_Thread; std::map > m_PendingInboundTunnels; // by replyMsgID + mutable std::mutex m_PendingInboundTunnelsMutex; std::map > m_PendingOutboundTunnels; // by replyMsgID + mutable std::mutex m_PendingOutboundTunnelsMutex; std::list > m_InboundTunnels; + mutable std::recursive_mutex m_InboundTunnelsMutex; std::list > m_OutboundTunnels; + mutable std::recursive_mutex m_OutboundTunnelsMutex; std::list > m_TransitTunnels; + mutable std::mutex m_TransitTunnelsMutex; std::unordered_map > m_Tunnels; // tunnelID->tunnel known by this id - std::mutex m_PoolsMutex; + mutable std::recursive_mutex m_TunnelsMutex; + mutable std::mutex m_PoolsMutex; std::list> m_Pools; std::shared_ptr m_ExploratoryPool; i2p::util::Queue > m_Queue;