More mutex refactoring.

Move some of the global lock debugging tools into a class that can be used
for more fine-grained locks.

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_lowlevel.cpp

@@ -57,10 +57,8 @@ constexpr SteamNetworkingMicroseconds k_usecDefaultLongLockHeldWarningThreshold
 int g_nSteamDatagramSocketBufferSize = 256*1024;
 
 /// Global lock for all local data structures
-static RecursiveTimedMutex s_steamDatagramTransportMutex;
-int SteamNetworkingGlobalLock::s_nLocked;
+static DebugMutex<RecursiveTimedMutexImpl> s_mutexGlobalLock;
 static SteamNetworkingMicroseconds s_usecWhenLocked;
-static std::thread::id s_threadIDLockOwner;
 static SteamNetworkingMicroseconds s_usecLongLockWarningThreshold;
 static SteamNetworkingMicroseconds s_usecIgnoreLongLockWaitTimeUntil;
 static int s_nCurrentLockTags;
@@ -92,13 +90,11 @@ void SteamNetworkingGlobalLock::AddTag( const char *pszTag )
 
 void SteamNetworkingGlobalLock::OnLocked( const char *pszTag, SteamNetworkingMicroseconds usecTimeStartedLocking )
 {
-	++s_nLocked;
 	SteamNetworkingMicroseconds usecNow = SteamNetworkingSockets_GetLocalTimestamp();
 	SteamNetworkingMicroseconds usecTimeSpentWaitingOnLock = usecNow - usecTimeStartedLocking;
-	if ( s_nLocked == 1 )
+	if ( s_mutexGlobalLock.m_nLockCount == 1 )
 	{
 		s_usecWhenLocked = usecNow;
-		s_threadIDLockOwner = std::this_thread::get_id();
 		s_usecLongLockWarningThreshold = k_usecDefaultLongLockHeldWarningThreshold;
 		s_nCurrentLockTags = 0;
 
@@ -117,9 +113,6 @@ void SteamNetworkingGlobalLock::OnLocked( const char *pszTag, SteamNetworkingMic
 	}
 	else
 	{
-		// This thread already held the lock
-		Assert( s_threadIDLockOwner == std::this_thread::get_id() );
-
 		// Getting it again had better be nearly instantaneous!
 		// FIXME I don't know why this is firing with a lower threshold.  What is it doing?
 		AssertMsg1( usecTimeSpentWaitingOnLock < 2000, "Waited %lldusec to take second lock on the same thread??", (long long)usecTimeSpentWaitingOnLock );
@@ -130,14 +123,14 @@ void SteamNetworkingGlobalLock::OnLocked( const char *pszTag, SteamNetworkingMic
 void SteamNetworkingGlobalLock::Lock( const char *pszTag )
 {
 	SteamNetworkingMicroseconds usecTimeStartedLocking = SteamNetworkingSockets_GetLocalTimestamp();
-	s_steamDatagramTransportMutex.lock();
+	s_mutexGlobalLock.lock();
 	OnLocked( pszTag, usecTimeStartedLocking );
 }
 
 bool SteamNetworkingGlobalLock::TryLock( const char *pszTag, int msTimeout )
 {
 	SteamNetworkingMicroseconds usecTimeStartedLocking = SteamNetworkingSockets_GetLocalTimestamp();
-	if ( !s_steamDatagramTransportMutex.try_lock_for( std::chrono::milliseconds( msTimeout ) ) )
+	if ( !s_mutexGlobalLock.try_lock_for( msTimeout ) )
 		return false;
 	OnLocked( pszTag, usecTimeStartedLocking );
 	return true;
@@ -152,7 +145,7 @@ void SteamNetworkingGlobalLock::Unlock()
 	SteamNetworkingMicroseconds usecElapsedTooLong = 0;
 	auto lockHeldCallback = s_fLockHeldCallback;
 
-	if ( s_nLocked == 1 )
+	if ( s_mutexGlobalLock.m_nLockCount == 1 )
 	{
 
 		// We're about to do the final release.  How long did we hold the lock?
@@ -194,12 +187,7 @@ void SteamNetworkingGlobalLock::Unlock()
 
 		s_nCurrentLockTags = 0;
 	}
-	--s_nLocked;
-	#ifdef MSVC_STL_MUTEX_WORKAROUND
-		DbgVerify( ReleaseMutex( s_hSteamDatagramTransportMutex ) );
-	#else
-		s_steamDatagramTransportMutex.unlock();
-	#endif
+	s_mutexGlobalLock.unlock();
 
 	if ( usecElapsed > 0 && lockHeldCallback )
 	{
@@ -227,14 +215,13 @@ void SteamNetworkingGlobalLock::SetLongLockWarningThresholdMS( const char *pszTa
 
 void SteamNetworkingGlobalLock::AssertHeldByCurrentThread()
 {
-	Assert( s_nLocked > 0 ); // NOTE: This could succeed even if another thread has the lock
-	Assert( s_threadIDLockOwner == std::this_thread::get_id() );
+	s_mutexGlobalLock.AssertHeldByCurrentThread();
 }
 
 void SteamNetworkingGlobalLock::AssertHeldByCurrentThread( const char *pszTag )
 {
-	Assert( s_nLocked > 0 ); // NOTE: This could succeed even if another thread has the lock
-	if ( s_threadIDLockOwner == std::this_thread::get_id() )
+	Assert( s_mutexGlobalLock.m_nLockCount > 0 ); // NOTE: This could succeed even if another thread has the lock
+	if ( s_mutexGlobalLock.m_threadIDLockOwner == std::this_thread::get_id() )
 	{
 		AddTag( pszTag );
 	}
@@ -1168,7 +1155,7 @@ static bool PollRawUDPSockets( int nMaxTimeoutMS, bool bManualPoll )
 	// This should only ever be called from our one thread proc,
 	// and we assume that it will have locked the lock exactly once.
 	SteamNetworkingGlobalLock::AssertHeldByCurrentThread();
-	Assert( SteamNetworkingGlobalLock::s_nLocked == 1 );
+	Assert( s_mutexGlobalLock.m_nLockCount == 1 );
 
 	const int nSocketsToPoll = s_vecRawSockets.Count();
 
@@ -1448,7 +1435,7 @@ void ProcessPendingDestroyClosedRawUDPSockets()
 static bool SteamNetworkingSockets_InternalPoll( int msWait, bool bManualPoll )
 {
 	SteamNetworkingGlobalLock::AssertHeldByCurrentThread(); // We should own the lock
-	Assert( SteamNetworkingGlobalLock::s_nLocked == 1 ); // exactly once
+	Assert( s_mutexGlobalLock.m_nLockCount == 1 ); // exactly once
 
 	// Figure out how long to sleep
 	IThinker *pNextThinker = Thinker_GetNextScheduled();
@@ -1503,7 +1490,7 @@ static bool SteamNetworkingSockets_InternalPoll( int msWait, bool bManualPoll )
 	}
 
 	SteamNetworkingGlobalLock::AssertHeldByCurrentThread(); // We should own the lock
-	Assert( SteamNetworkingGlobalLock::s_nLocked == 1 ); // exactly once
+	Assert( s_mutexGlobalLock.m_nLockCount == 1 ); // exactly once
 
 	// Shutdown request?
 	if ( s_nLowLevelSupportRefCount.load(std::memory_order_acquire) <= 0 || s_bManualPollMode != bManualPoll )

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_lowlevel.h

@@ -8,6 +8,7 @@
 #include <cstdint>
 #include <functional>
 #include <mutex>
+#include <thread>
 #include <steam/steamnetworkingtypes.h>
 #include <tier1/netadr.h>
 #include <tier1/utlhashmap.h>
@@ -24,9 +25,49 @@ namespace SteamNetworkingSocketsLib {
 
 // You can override these with more optimal platform-specific
 // versions if you want
-using Mutex = std::mutex; // Spinlock - no recursion, no timeout
-using RecursiveMutex = std::recursive_mutex; // Recursion, but no timeout
-using RecursiveTimedMutex = std::recursive_timed_mutex; // Recursion, but no timeout
+using MutexImpl = std::mutex; // Spinlock - no recursion, no timeout
+using RecursiveMutexImpl = std::recursive_mutex; // Recursion, but no timeout
+using RecursiveTimedMutexImpl = std::recursive_timed_mutex; // Recursion, but no timeout
+
+/// Wrapper for locks to make them somewhat debuggable.
+template<typename TMutexImpl>
+struct DebugMutex
+{
+	inline void lock()
+	{
+		m_impl.lock();
+		OnLocked();
+	}
+	inline void unlock() { AssertHeldByCurrentThread(); --m_nLockCount; m_impl.unlock(); }
+	inline bool try_lock() { if ( !m_impl.try_lock() ) return false; OnLocked(); return true; }
+	inline bool try_lock_for( int msTimeout ) { if ( !m_impl.try_lock_for( std::chrono::milliseconds( msTimeout ) ) ) return false; OnLocked(); return true; }
+
+	inline void AssertHeldByCurrentThread()
+	{
+		Assert( m_nLockCount > 0 ); // Super fast, but subject to false negative (Won't properly fail if another thread has the lock).  Should catch most mistakes though, because often no thread has the lock.
+		DbgAssert( m_threadIDLockOwner == std::this_thread::get_id() ); // Doesn't catch as many bugs, only run in debug
+	}
+	volatile int m_nLockCount = 0;
+	std::thread::id m_threadIDLockOwner;
+private:
+	void OnLocked()
+	{
+		++m_nLockCount;
+		if ( m_nLockCount == 1 )
+		{
+			m_threadIDLockOwner = std::this_thread::get_id();
+		}
+		else
+		{
+			DbgAssert( m_threadIDLockOwner == std::this_thread::get_id() );
+		}
+	}
+	TMutexImpl m_impl;
+};
+
+using Mutex = DebugMutex<MutexImpl>;
+using RecursiveMutex = DebugMutex<RecursiveMutexImpl>;
+using RecursiveTimedMutex = DebugMutex<RecursiveTimedMutexImpl>;
 
 /////////////////////////////////////////////////////////////////////////////
 //
@@ -322,7 +363,6 @@ struct SteamNetworkingGlobalLock
 	static void AssertHeldByCurrentThread( const char *pszTag );
 	static void SetLongLockWarningThresholdMS( const char *pszTag, int msWarningThreshold );
 	static void AddTag( const char *pszTag );
-	static int s_nLocked;
 private:
 	static void OnLocked( const char *pszTag, SteamNetworkingMicroseconds usecTimeStartedLocking );
 };