Revert "Merge pull request #1703 from simonvetter/simon/memory-and-multithreading-fixes"

This reverts commit 67863cfcf9, reversing
changes made to 4c5ec68ff1.

That change completly bloking transports thread on windows.

Signed-off-by: R4SAS <r4sas@i2pmail.org>

libi2pd/Tunnel.cpp

@@ -366,7 +366,6 @@ namespace tunnel
 
 	std::shared_ptr<TunnelBase> Tunnels::GetTunnel (uint32_t tunnelID)
 	{
-		std::unique_lock<std::recursive_mutex> l(m_TunnelsMutex);
 		auto it = m_Tunnels.find(tunnelID);
 		if (it != m_Tunnels.end ())
 			return it->second;
@@ -375,13 +374,11 @@ namespace tunnel
 
 	std::shared_ptr<InboundTunnel> Tunnels::GetPendingInboundTunnel (uint32_t replyMsgID)
 	{
-		std::unique_lock<std::mutex> l(m_PendingInboundTunnelsMutex);
 		return GetPendingTunnel (replyMsgID, m_PendingInboundTunnels);
 	}
 
 	std::shared_ptr<OutboundTunnel> Tunnels::GetPendingOutboundTunnel (uint32_t replyMsgID)
 	{
-		std::unique_lock<std::mutex> l(m_PendingOutboundTunnelsMutex);
 		return GetPendingTunnel (replyMsgID, m_PendingOutboundTunnels);
 	}
 
@@ -462,11 +459,9 @@ namespace tunnel
 
 	void Tunnels::AddTransitTunnel (std::shared_ptr<TransitTunnel> tunnel)
 	{
-		std::unique_lock<std::recursive_mutex> l(m_TunnelsMutex);
+		if (m_Tunnels.emplace (tunnel->GetTunnelID (), tunnel).second)
-		if (m_Tunnels.emplace (tunnel->GetTunnelID (), tunnel).second) {
-			std::unique_lock<std::mutex> l(m_TransitTunnelsMutex);
 			m_TransitTunnels.push_back (tunnel);
-		} else
+		else
 			LogPrint (eLogError, "Tunnel: tunnel with id ", tunnel->GetTunnelID (), " already exists");
 	}
 
@@ -621,10 +616,7 @@ namespace tunnel
 
 	void Tunnels::ManagePendingTunnels ()
 	{
-		std::unique_lock<std::mutex> li(m_PendingInboundTunnelsMutex);
 		ManagePendingTunnels (m_PendingInboundTunnels);
-		li.unlock();
-		std::unique_lock<std::mutex> lo(m_PendingOutboundTunnelsMutex);
 		ManagePendingTunnels (m_PendingOutboundTunnels);
 	}
 
@@ -684,7 +676,6 @@ namespace tunnel
 
 	void Tunnels::ManageOutboundTunnels ()
 	{
-		std::unique_lock<std::recursive_mutex> l(m_OutboundTunnelsMutex);
 		uint64_t ts = i2p::util::GetSecondsSinceEpoch ();
 		{
 			for (auto it = m_OutboundTunnels.begin (); it != m_OutboundTunnels.end ();)
@@ -739,8 +730,6 @@ namespace tunnel
 
 	void Tunnels::ManageInboundTunnels ()
 	{
-		std::unique_lock<std::recursive_mutex> lt(m_TunnelsMutex);
-		std::unique_lock<std::recursive_mutex> li(m_InboundTunnelsMutex);
 		uint64_t ts = i2p::util::GetSecondsSinceEpoch ();
 		{
 			for (auto it = m_InboundTunnels.begin (); it != m_InboundTunnels.end ();)
@@ -797,7 +786,6 @@ namespace tunnel
 			return;
 		}
 
-		std::unique_lock<std::recursive_mutex> lo(m_OutboundTunnelsMutex);
 		if (m_OutboundTunnels.empty () || m_InboundTunnels.size () < 3)
 		{
 			// trying to create one more inbound tunnel
@@ -818,8 +806,6 @@ namespace tunnel
 
 	void Tunnels::ManageTransitTunnels ()
 	{
-		std::unique_lock<std::recursive_mutex> lt(m_TunnelsMutex);
-		std::unique_lock<std::mutex> l(m_TransitTunnelsMutex);
 		uint32_t ts = i2p::util::GetSecondsSinceEpoch ();
 		for (auto it = m_TransitTunnels.begin (); it != m_TransitTunnels.end ();)
 		{
@@ -890,20 +876,17 @@ namespace tunnel
 
 	void Tunnels::AddPendingTunnel (uint32_t replyMsgID, std::shared_ptr<InboundTunnel> tunnel)
 	{
-		std::unique_lock<std::mutex> l(m_PendingInboundTunnelsMutex);
 		m_PendingInboundTunnels[replyMsgID] = tunnel;
 	}
 
 	void Tunnels::AddPendingTunnel (uint32_t replyMsgID, std::shared_ptr<OutboundTunnel> tunnel)
 	{
-		std::unique_lock<std::mutex> l(m_PendingOutboundTunnelsMutex);
 		m_PendingOutboundTunnels[replyMsgID] = tunnel;
 	}
 
 	void Tunnels::AddOutboundTunnel (std::shared_ptr<OutboundTunnel> newTunnel)
 	{
 		// we don't need to insert it to m_Tunnels
-		std::unique_lock<std::recursive_mutex> l(m_OutboundTunnelsMutex);
 		m_OutboundTunnels.push_back (newTunnel);
 		auto pool = newTunnel->GetTunnelPool ();
 		if (pool && pool->IsActive ())
@@ -914,10 +897,8 @@ namespace tunnel
 
 	void Tunnels::AddInboundTunnel (std::shared_ptr<InboundTunnel> newTunnel)
 	{
-		std::unique_lock<std::recursive_mutex> l(m_TunnelsMutex);
 		if (m_Tunnels.emplace (newTunnel->GetTunnelID (), newTunnel).second)
 		{
-			std::unique_lock<std::recursive_mutex> l(m_InboundTunnelsMutex);
 			m_InboundTunnels.push_back (newTunnel);
 			auto pool = newTunnel->GetTunnelPool ();
 			if (!pool)
@@ -945,8 +926,6 @@ namespace tunnel
 		auto inboundTunnel = std::make_shared<ZeroHopsInboundTunnel> ();
 		inboundTunnel->SetTunnelPool (pool);
 		inboundTunnel->SetState (eTunnelStateEstablished);
-		std::unique_lock<std::recursive_mutex> lt(m_TunnelsMutex);
-		std::unique_lock<std::recursive_mutex> li(m_InboundTunnelsMutex);
 		m_InboundTunnels.push_back (inboundTunnel);
 		m_Tunnels[inboundTunnel->GetTunnelID ()] = inboundTunnel;
 		return inboundTunnel;
@@ -957,7 +936,6 @@ namespace tunnel
 		auto outboundTunnel = std::make_shared<ZeroHopsOutboundTunnel> ();
 		outboundTunnel->SetTunnelPool (pool);
 		outboundTunnel->SetState (eTunnelStateEstablished);
-		std::unique_lock<std::recursive_mutex> l(m_OutboundTunnelsMutex);
 		m_OutboundTunnels.push_back (outboundTunnel);
 		// we don't insert into m_Tunnels
 		return outboundTunnel;
@@ -986,7 +964,6 @@ namespace tunnel
 		int timeout = 0;
 		uint32_t ts = i2p::util::GetSecondsSinceEpoch ();
 		// TODO: possible race condition with I2PControl
-		std::unique_lock<std::mutex> l(m_TransitTunnelsMutex);
 		for (const auto& it : m_TransitTunnels)
 		{
 			int t = it->GetCreationTime () + TUNNEL_EXPIRATION_TIMEOUT - ts;
@@ -997,19 +974,19 @@ namespace tunnel
 
 	size_t Tunnels::CountTransitTunnels() const
 	{
-		std::unique_lock<std::mutex> l(m_TransitTunnelsMutex);
+		// TODO: locking
 		return m_TransitTunnels.size();
 	}
 
 	size_t Tunnels::CountInboundTunnels() const
 	{
-		std::unique_lock<std::recursive_mutex> l(m_InboundTunnelsMutex);
+		// TODO: locking
 		return m_InboundTunnels.size();
 	}
 
 	size_t Tunnels::CountOutboundTunnels() const
 	{
-		std::unique_lock<std::recursive_mutex> l(m_OutboundTunnelsMutex);
+		// TODO: locking
 		return m_OutboundTunnels.size();
 	}
 }

libi2pd/Tunnel.h

@@ -68,7 +68,7 @@ namespace tunnel
 		public:
 
 			Tunnel (std::shared_ptr<const TunnelConfig> config);
-			virtual ~Tunnel ();
+			~Tunnel ();
 
 			void Build (uint32_t replyMsgID, std::shared_ptr<OutboundTunnel> outboundTunnel = nullptr);
 
@@ -254,18 +254,12 @@ namespace tunnel
 			bool m_IsRunning;
 			std::thread * m_Thread;
 			std::map<uint32_t, std::shared_ptr<InboundTunnel> > m_PendingInboundTunnels; // by replyMsgID
-			mutable std::mutex m_PendingInboundTunnelsMutex;
 			std::map<uint32_t, std::shared_ptr<OutboundTunnel> > m_PendingOutboundTunnels; // by replyMsgID
-			mutable std::mutex m_PendingOutboundTunnelsMutex;
 			std::list<std::shared_ptr<InboundTunnel> > m_InboundTunnels;
-			mutable std::recursive_mutex m_InboundTunnelsMutex;
 			std::list<std::shared_ptr<OutboundTunnel> > m_OutboundTunnels;
-			mutable std::recursive_mutex m_OutboundTunnelsMutex;
 			std::list<std::shared_ptr<TransitTunnel> > m_TransitTunnels;
-			mutable std::mutex m_TransitTunnelsMutex;
 			std::unordered_map<uint32_t, std::shared_ptr<TunnelBase> > m_Tunnels; // tunnelID->tunnel known by this id
-			mutable std::recursive_mutex m_TunnelsMutex;
+			std::mutex m_PoolsMutex;
-			mutable std::mutex m_PoolsMutex;
 			std::list<std::shared_ptr<TunnelPool>> m_Pools;
 			std::shared_ptr<TunnelPool> m_ExploratoryPool;
 			i2p::util::Queue<std::shared_ptr<I2NPMessage> > m_Queue;

libi2pd/TunnelConfig.h

@@ -99,7 +99,7 @@ namespace tunnel
 				m_LastHop->SetReplyHop (replyTunnelID, replyIdent);
 			}
 
-			virtual ~TunnelConfig ()
+			~TunnelConfig ()
 			{
 				TunnelHopConfig * hop = m_FirstHop;
 

libi2pd/TunnelPool.cpp

@@ -59,7 +59,6 @@ namespace tunnel
 
 	void TunnelPool::SetExplicitPeers (std::shared_ptr<std::vector<i2p::data::IdentHash> > explicitPeers)
 	{
-		std::unique_lock<std::mutex> l(m_ExplicitPeersMutex);
 		m_ExplicitPeers = explicitPeers;
 		if (m_ExplicitPeers)
 		{
@@ -93,7 +92,6 @@ namespace tunnel
 				it->SetTunnelPool (nullptr);
 			m_OutboundTunnels.clear ();
 		}
-		std::unique_lock<std::mutex> l(m_TestsMutex);
 		m_Tests.clear ();
 	}
 
@@ -137,11 +135,10 @@ namespace tunnel
 		if (expiredTunnel)
 		{
 			expiredTunnel->SetTunnelPool (nullptr);
-			std::unique_lock<std::mutex> lt(m_TestsMutex);
 			for (auto& it: m_Tests)
 				if (it.second.second == expiredTunnel) it.second.second = nullptr;
 
-			std::unique_lock<std::mutex> li(m_InboundTunnelsMutex);
+			std::unique_lock<std::mutex> l(m_InboundTunnelsMutex);
 			m_InboundTunnels.erase (expiredTunnel);
 		}
 	}
@@ -160,11 +157,10 @@ namespace tunnel
 		if (expiredTunnel)
 		{
 			expiredTunnel->SetTunnelPool (nullptr);
-			std::unique_lock<std::mutex> lt(m_TestsMutex);
 			for (auto& it: m_Tests)
 				if (it.second.first == expiredTunnel) it.second.first = nullptr;
 
-			std::unique_lock<std::mutex> lo(m_OutboundTunnelsMutex);
+			std::unique_lock<std::mutex> l(m_OutboundTunnelsMutex);
 			m_OutboundTunnels.erase (expiredTunnel);
 		}
 	}
@@ -269,7 +265,7 @@ namespace tunnel
 	{
 		int num = 0;
 		{
-			std::unique_lock<std::mutex> lo(m_OutboundTunnelsMutex);
+			std::unique_lock<std::mutex> l(m_OutboundTunnelsMutex);
 			for (const auto& it : m_OutboundTunnels)
 				if (it->IsEstablished ()) num++;
 		}
@@ -278,11 +274,10 @@ namespace tunnel
 
 		num = 0;
 		{
-			std::unique_lock<std::mutex> li(m_InboundTunnelsMutex);
+			std::unique_lock<std::mutex> l(m_InboundTunnelsMutex);
 			for (const auto& it : m_InboundTunnels)
 				if (it->IsEstablished ()) num++;
 		}
-		std::unique_lock<std::mutex> lo(m_OutboundTunnelsMutex);
 		if (!num && !m_OutboundTunnels.empty () && m_NumOutboundHops > 0)
 		{
 			for (auto it: m_OutboundTunnels)
@@ -292,7 +287,6 @@ namespace tunnel
 				if (num >= m_NumInboundTunnels) break;
 			}	
 		}	
-		lo.unlock();
 		for (int i = num; i < m_NumInboundTunnels; i++)
 			CreateInboundTunnel ();
 
@@ -317,7 +311,7 @@ namespace tunnel
 				if (it.second.first->GetState () == eTunnelStateTestFailed)
 				{
 					it.second.first->SetState (eTunnelStateFailed);
-					std::unique_lock<std::mutex> lo(m_OutboundTunnelsMutex);
+					std::unique_lock<std::mutex> l(m_OutboundTunnelsMutex);
 					m_OutboundTunnels.erase (it.second.first);
 				}
 				else
@@ -329,7 +323,7 @@ namespace tunnel
 				{
 					it.second.second->SetState (eTunnelStateFailed);
 					{
-						std::unique_lock<std::mutex> li(m_InboundTunnelsMutex);
+						std::unique_lock<std::mutex> l(m_InboundTunnelsMutex);
 						m_InboundTunnels.erase (it.second.second);
 					}
 					if (m_LocalDestination)
@@ -341,10 +335,7 @@ namespace tunnel
 		}
 
 		// new tests
-		std::unique_lock<std::mutex> lt(m_TestsMutex);
-		std::unique_lock<std::mutex> lo(m_OutboundTunnelsMutex);
 		auto it1 = m_OutboundTunnels.begin ();
-		std::unique_lock<std::mutex> li(m_InboundTunnelsMutex);
 		auto it2 = m_InboundTunnels.begin ();
 		while (it1 != m_OutboundTunnels.end () && it2 != m_InboundTunnels.end ())
 		{
@@ -364,6 +355,7 @@ namespace tunnel
 				uint32_t msgID;
 				RAND_bytes ((uint8_t *)&msgID, 4);
 				{
+					std::unique_lock<std::mutex> l(m_TestsMutex);
 					m_Tests[msgID] = std::make_pair (*it1, *it2);
 				}
 				(*it1)->SendTunnelDataMsg ((*it2)->GetNextIdentHash (), (*it2)->GetNextTunnelID (),
@@ -388,7 +380,7 @@ namespace tunnel
 		if (m_LocalDestination)
 			m_LocalDestination->ProcessGarlicMessage (msg);
 		else
-			LogPrint (eLogWarning, "Tunnels: local destination doesn't exist, garlic message dropped");
+			LogPrint (eLogWarning, "Tunnels: local destination doesn't exist, dropped");
 	}
 
 	void TunnelPool::ProcessDeliveryStatus (std::shared_ptr<I2NPMessage> msg)
@@ -436,7 +428,7 @@ namespace tunnel
 			if (m_LocalDestination)
 				m_LocalDestination->ProcessDeliveryStatusMessage (msg);
 			else
-				LogPrint (eLogWarning, "Tunnels: Local destination doesn't exist, delivery status message dropped");
+				LogPrint (eLogWarning, "Tunnels: Local destination doesn't exist, dropped");
 		}
 	}
 
@@ -520,16 +512,13 @@ namespace tunnel
 			if (m_CustomPeerSelector)
 				return m_CustomPeerSelector->SelectPeers(path, numHops, isInbound);
 		}
-
 		// explicit peers in use
-		std::lock_guard<std::mutex> lock(m_ExplicitPeersMutex);
 		if (m_ExplicitPeers) return SelectExplicitPeers (path, isInbound);
 		return StandardSelectPeers(path, numHops, isInbound, std::bind(&TunnelPool::SelectNextHop, this, std::placeholders::_1, std::placeholders::_2));
 	}
 
 	bool TunnelPool::SelectExplicitPeers (Path& path, bool isInbound)
 	{
-		std::unique_lock<std::mutex> l(m_ExplicitPeersMutex);
 		int numHops = isInbound ? m_NumInboundHops : m_NumOutboundHops;
 		if (numHops > (int)m_ExplicitPeers->size ()) numHops = m_ExplicitPeers->size ();
 		if (!numHops) return false; 

libi2pd/TunnelPool.h

@@ -128,7 +128,6 @@ namespace tunnel
 
 			std::shared_ptr<i2p::garlic::GarlicDestination> m_LocalDestination;
 			int m_NumInboundHops, m_NumOutboundHops, m_NumInboundTunnels, m_NumOutboundTunnels;
-			mutable std::mutex m_ExplicitPeersMutex;
 			std::shared_ptr<std::vector<i2p::data::IdentHash> > m_ExplicitPeers;
 			mutable std::mutex m_InboundTunnelsMutex;
 			std::set<std::shared_ptr<InboundTunnel>, TunnelCreationTimeCmp> m_InboundTunnels; // recent tunnel appears first