From e9275beacb185c26af6ef431bf2ea28cf690f601 Mon Sep 17 00:00:00 2001
From: Fletcher Dunn <fletcherd@valvesoftware.com>
Date: Fri, 28 May 2021 17:37:22 -0700
Subject: [PATCH] [Steam only] Fix locking issues

Related to the legacy poll group when destroying listen sockets.
(We don't need this legacy support in this library, since we don't
have to support old interfaces.)

P4:6571400
---
 .../clientlib/csteamnetworkingsockets.cpp     |  4 +++-
 .../steamnetworkingsockets_connections.cpp    | 23 +++++++++++++++----
 .../steamnetworkingsockets_connections.h      |  2 +-
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp b/src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp
index 9b9a073..f2fe560 100644
--- a/src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp
+++ b/src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp
@@ -1298,8 +1298,10 @@ int CSteamNetworkingSockets::ReceiveMessagesOnListenSocketLegacyPollGroup( HStea
 	CSteamNetworkListenSocketBase *pSock = GetListenSocketByHandle( hSocket );
 	if ( !pSock )
 		return -1;
+	if ( !pSock->m_pLegacyPollGroup )
+		return 0;
 	g_lockAllRecvMessageQueues.lock();
-	int nMessagesReceived = pSock->m_legacyPollGroup.m_queueRecvMessages.RemoveMessages( ppOutMessages, nMaxMessages );
+	int nMessagesReceived = pSock->m_pLegacyPollGroup->m_queueRecvMessages.RemoveMessages( ppOutMessages, nMaxMessages );
 	g_lockAllRecvMessageQueues.unlock();
 	return nMessagesReceived;
 }
diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp
index f078b41..ed651e8 100644
--- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp
+++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp
@@ -346,7 +346,6 @@ CSteamNetworkPollGroup::~CSteamNetworkPollGroup()
 	// Object deletion is rare; to keep things simple we require the global lock
 	SteamNetworkingGlobalLock::AssertHeldByCurrentThread();
 	m_lock.AssertHeldByCurrentThread();
-	g_tables_lock.AssertHeldByCurrentThread();
 
 	FOR_EACH_VEC_BACK( m_vecConnections, i )
 	{
@@ -385,6 +384,7 @@ CSteamNetworkPollGroup::~CSteamNetworkPollGroup()
 	// Remove us from global table, if we're in it
 	if ( m_hPollGroupSelf != k_HSteamNetPollGroup_Invalid )
 	{
+		g_tables_lock.AssertHeldByCurrentThread();
 		int idx = m_hPollGroupSelf & 0xffff;
 		if ( g_mapPollGroups.IsValidIndex( idx ) && g_mapPollGroups[ idx ] == this )
 		{
@@ -443,9 +443,6 @@ void CSteamNetworkPollGroup::AssignHandleAndAddToGlobalTable()
 CSteamNetworkListenSocketBase::CSteamNetworkListenSocketBase( CSteamNetworkingSockets *pSteamNetworkingSocketsInterface )
 : m_pSteamNetworkingSocketsInterface( pSteamNetworkingSocketsInterface )
 , m_hListenSocketSelf( k_HSteamListenSocket_Invalid )
-#ifdef STEAMNETWORKINGSOCKETS_STEAMCLIENT
-, m_legacyPollGroup( pSteamNetworkingSocketsInterface )
-#endif
 {
 	m_connectionConfig.Init( &pSteamNetworkingSocketsInterface->m_connectionConfig );
 }
@@ -473,6 +470,10 @@ CSteamNetworkListenSocketBase::~CSteamNetworkListenSocketBase()
 
 		m_hListenSocketSelf = k_HSteamListenSocket_Invalid;
 	}
+
+	#ifdef STEAMNETWORKINGSOCKETS_STEAMCLIENT
+		Assert( !m_pLegacyPollGroup ); // Should have been cleaned up by Destroy()
+	#endif
 }
 
 bool CSteamNetworkListenSocketBase::BInitListenSocketCommon( int nOptions, const SteamNetworkingConfigValue_t *pOptions, SteamDatagramErrMsg &errMsg )
@@ -556,6 +557,14 @@ void CSteamNetworkListenSocketBase::Destroy()
 		Assert( m_mapChildConnections.Count() == n-1 );
 	}
 
+	#ifdef STEAMNETWORKINGSOCKETS_STEAMCLIENT
+	if ( m_pLegacyPollGroup )
+	{
+		m_pLegacyPollGroup->m_lock.lock(); // Don't use scope object.  It will unlock when we destruct
+		m_pLegacyPollGroup.reset();
+	}
+	#endif
+
 	// Self destruct
 	delete this;
 }
@@ -615,7 +624,11 @@ bool CSteamNetworkListenSocketBase::BAddChildConnection( CSteamNetworkConnection
 	// Don't override it, if so.)
 	#ifdef STEAMNETWORKINGSOCKETS_STEAMCLIENT
 	if ( !pConn->m_pPollGroup )
-		pConn->SetPollGroup( &m_legacyPollGroup );
+	{
+		if ( !m_pLegacyPollGroup )
+			m_pLegacyPollGroup.reset( new CSteamNetworkPollGroup( m_pSteamNetworkingSocketsInterface ) );
+		pConn->SetPollGroup( m_pLegacyPollGroup.get() );
+	}
 	#endif
 
 	return true;
diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h
index 7392764..981aa12 100644
--- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h
+++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h
@@ -264,7 +264,7 @@ public:
 
 	/// For legacy interface.
 	#ifdef STEAMNETWORKINGSOCKETS_STEAMCLIENT
-	CSteamNetworkPollGroup m_legacyPollGroup;
+	std::unique_ptr<CSteamNetworkPollGroup> m_pLegacyPollGroup;
 	#endif
 
 protected: