[engine] Fix sv_parallel_sendsnapshot crashing

engine/baseserver.cpp

@@ -2231,145 +2231,130 @@ void CBaseServer::BroadcastMessage( INetMessage &msg, IRecipientFilter &filter )
 //-----------------------------------------------------------------------------
 static ConVar sv_debugtempentities( "sv_debugtempentities", "0", 0, "Show temp entity bandwidth usage." );
 
-static bool CEventInfo_LessFunc( CEventInfo * const &lhs, CEventInfo * const &rhs )
-{
-	return lhs->classID < rhs->classID;
-}
+// 8 KB should be far more than is needed -- in fact less than 100 bytes seems to be sufficient
+const int kTempEntityBufferSize = 8192;
 
 void CBaseServer::WriteTempEntities( CBaseClient *client, CFrameSnapshot *pCurrentSnapshot, CFrameSnapshot *pLastSnapshot, bf_write &buf, int ev_max )
 {
 	VPROF_BUDGET( "CBaseServer::WriteTempEntities", VPROF_BUDGETGROUP_OTHER_NETWORKING );
 
-	ALIGN4 char data[NET_MAX_PAYLOAD] ALIGN4_POST;
+	ALIGN4 char data[kTempEntityBufferSize] ALIGN4_POST;
 	SVC_TempEntities msg;
 	msg.m_DataOut.StartWriting( data, sizeof(data) );
 	bf_write &buffer = msg.m_DataOut; // shortcut
-
-	CFrameSnapshot *pSnapshot;
-	CEventInfo *pLastEvent = NULL;
 
 	bool bDebug = sv_debugtempentities.GetBool();
 
-	// limit max entities to field bit length
-	ev_max = min( ev_max, ((1<<CEventInfo::EVENT_INDEX_BITS)-1) );
+	// Container which calls ReleaseReference on all snapshots in list on exit of function scope
+	CReferencedSnapshotList snapshotlist;
+	// Builds list and calls AddReference on each item in list (uses a mutex to be thread safe)
+	framesnapshotmanager->BuildSnapshotList( pCurrentSnapshot, pLastSnapshot, CFrameSnapshotManager::knDefaultSnapshotSet, snapshotlist );
 
-	if ( pLastSnapshot )
-	{
-		pSnapshot = pLastSnapshot->NextSnapshot();
-	} 
-	else
-	{
-		pSnapshot = pCurrentSnapshot;
-	}
-
-	CUtlRBTree< CEventInfo * >	sorted( 0, ev_max, CEventInfo_LessFunc );
+	//keep count of the number of entities that we write out (since some can be omitted by the client)
+	int32 nNumEntitiesWritten = 0;
+	CEventInfo *pLastEvent = NULL;
 
 	// Build list of events sorted by send table classID (makes the delta work better in cases with a lot of the same message type )
-	while ( pSnapshot && ((int)sorted.Count() < ev_max) )
+	for ( int nSnapShotIndex = 0; 
+		 nSnapShotIndex < snapshotlist.m_vecSnapshots.Count(); 
+		 ++nSnapShotIndex )
 	{
+		CFrameSnapshot *pSnapshot = snapshotlist.m_vecSnapshots[ nSnapShotIndex ];
+
 		for( int i = 0; i < pSnapshot->m_nTempEntities; ++i )
 		{
 			CEventInfo *event = pSnapshot->m_pTempEntities[ i ];
 
 			if ( client->IgnoreTempEntity( event ) )
 				continue; // event is not seen by this player
-
-			sorted.Insert( event );
-			// More space still
-			if ( (int)sorted.Count() >= ev_max )
-				break;	
-		}
-
-		// stop, we reached our current snapshot
-		if ( pSnapshot == pCurrentSnapshot )
-			break; 
-
-		// got to next snapshot
-		pSnapshot = framesnapshotmanager->NextSnapshot( pSnapshot );
-	}
-
-	if ( sorted.Count() <= 0 )
-		return;
 
-	for ( auto i = sorted.FirstInorder(); 
-		i != sorted.InvalidIndex(); 
-		i = sorted.NextInorder( i ) )
-	{
-		CEventInfo *event = sorted[ i ];
+			//we are writing this entity so update our count so the receiver knows how many to parse
+			nNumEntitiesWritten++;
 
-		if ( event->fire_delay == 0.0f )
-		{
-			buffer.WriteOneBit( 0 );
-		} 
-		else
-		{
-			buffer.WriteOneBit( 1 );
-			buffer.WriteSBitLong( event->fire_delay*100.0f, 8 );
-		}
-
-		if ( pLastEvent && 
-			pLastEvent->classID == event->classID )
-		{
-			buffer.WriteOneBit( 0 ); // delta against last temp entity
-
-			int startBit = bDebug ? buffer.GetNumBitsWritten() : 0;
-
-			SendTable_WriteAllDeltaProps( event->pSendTable, 
-				pLastEvent->pData,
-				pLastEvent->bits,
-				event->pData,
-				event->bits,
-				-1,
-				&buffer );
-
-			if ( bDebug )
+			if ( event->fire_delay == 0.0f )
+			{
+				buffer.WriteOneBit( 0 );
+			} 
+			else
 			{
-				int length = buffer.GetNumBitsWritten() - startBit;
-				DevMsg("TE %s delta bits: %i\n", event->pSendTable->GetName(), length );
+				buffer.WriteOneBit( 1 );
+				buffer.WriteSBitLong( event->fire_delay*100.0f, 8 );
 			}
-		}
-		else
-		{
-			 // full update, just compressed against zeros in MP
-
-			buffer.WriteOneBit( 1 );
 
-			int startBit = bDebug ? buffer.GetNumBitsWritten() : 0;
+			if ( pLastEvent && 
+				pLastEvent->classID == event->classID )
+			{
+				buffer.WriteOneBit( 0 ); // delta against last temp entity
 
-			buffer.WriteUBitLong( event->classID, GetClassBits() );
+				int startBit = bDebug ? buffer.GetNumBitsWritten() : 0;
 
-			if ( IsMultiplayer() )
-			{
 				SendTable_WriteAllDeltaProps( event->pSendTable, 
-					NULL,	// will write only non-zero elements
-					0,
+					pLastEvent->pData,
+					pLastEvent->bits,
 					event->pData,
 					event->bits,
 					-1,
 					&buffer );
+
+				if ( bDebug )
+				{
+					int length = buffer.GetNumBitsWritten() - startBit;
+					DevMsg("TE %s delta bits: %i\n", event->pSendTable->GetName(), length );
+				}
 			}
 			else
 			{
-				// write event with zero properties
-				buffer.WriteBits( event->pData, event->bits );
+				 // full update, just compressed against zeros in MP
+
+				buffer.WriteOneBit( 1 );
+
+				int startBit = bDebug ? buffer.GetNumBitsWritten() : 0;
+
+				buffer.WriteUBitLong( event->classID, GetClassBits() );
+
+				if ( IsMultiplayer() )
+				{
+					SendTable_WriteAllDeltaProps( event->pSendTable, 
+						NULL,	// will write only non-zero elements
+						0,
+						event->pData,
+						event->bits,
+						-1,
+						&buffer );
+				}
+				else
+				{
+					// write event with zero properties
+					buffer.WriteBits( event->pData, event->bits );
+				}
+
+				if ( bDebug )
+				{
+					int length = buffer.GetNumBitsWritten() - startBit;
+					DevMsg("TE %s full bits: %i\n", event->pSendTable->GetName(), length );
+				}
 			}
 
-			if ( bDebug )
+			if ( IsMultiplayer() )
 			{
-				int length = buffer.GetNumBitsWritten() - startBit;
-				DevMsg("TE %s full bits: %i\n", event->pSendTable->GetName(), length );
+				// in single player, don't used delta compression, lastEvent remains NULL
+				pLastEvent = event;
 			}
 		}
+	}
 
-		if ( IsMultiplayer() )
-		{
-			// in single player, don't used delta compression, lastEvent remains NULL
-			pLastEvent = event;
-		}
+	//don't do any more work if we didn't write anything out
+	if( nNumEntitiesWritten <= 0 )
+		return;
+
+	if ( buffer.IsOverflowed() )
+	{
+		Warning( "WriteTempOverflow! Discarding all ents!\n" );
+		return;
 	}
 
 	// set num entries
-	msg.m_nNumEntries = sorted.Count();
+	msg.m_nNumEntries = nNumEntitiesWritten;
 	msg.WriteToBuffer( buf );
 }
 

engine/framesnapshot.h

@@ -106,11 +106,30 @@ class CFrameSnapshot
 	CUtlVector<int>			m_iExplicitDeleteSlots;
 
 private:
+	friend class CFrameSnapshotManager;
+
+	//the set that this snapshot belongs to
+	uint32					m_nSnapshotSet;
 
 	// Snapshots auto-delete themselves when their refcount goes to zero.
 	CInterlockedInt			m_nReferences;
 };
 
+class CReferencedSnapshotList
+{
+public:
+	~CReferencedSnapshotList()
+	{
+		for ( int i = 0; i < m_vecSnapshots.Count(); ++i )
+		{
+			m_vecSnapshots[ i ]->ReleaseReference();
+		}
+		m_vecSnapshots.RemoveAll();
+	}
+
+	CUtlVector< CFrameSnapshot * > m_vecSnapshots;
+};
+
 //-----------------------------------------------------------------------------
 // Purpose: snapshot manager class
 //-----------------------------------------------------------------------------
@@ -125,14 +144,16 @@ class CFrameSnapshotManager
 
 	// IFrameSnapshot implementation.
 public:
+	//the default identifier to use for snapshots that are created
+	static const uint32 knDefaultSnapshotSet = 0;
 
 	// Called when a level change happens
 	virtual void			LevelChanged();
 
 	// Called once per frame after simulation to store off all entities.
 	// Note: the returned snapshot has a recount of 1 so you MUST call ReleaseReference on it.
-	CFrameSnapshot*	CreateEmptySnapshot( int ticknumber, int maxEntities );
-	CFrameSnapshot*	TakeTickSnapshot( int ticknumber );
+	CFrameSnapshot*	CreateEmptySnapshot( int ticknumber, int maxEntities, uint32 nSnapshotSet = knDefaultSnapshotSet );
+	CFrameSnapshot*	TakeTickSnapshot( int ticknumber, uint32 nSnapshotSet = knDefaultSnapshotSet );
 
 	CFrameSnapshot*	NextSnapshot( const CFrameSnapshot *pSnapshot );
 
@@ -163,9 +184,12 @@ class CFrameSnapshotManager
 	// List of entities to explicitly delete
 	void			AddExplicitDelete( int iSlot );
 
+	void			BuildSnapshotList( CFrameSnapshot *pCurrentSnapshot, CFrameSnapshot *pLastSnapshot, uint32 nSnapshotSet, CReferencedSnapshotList &list );
+
 private:
 	void	DeleteFrameSnapshot( CFrameSnapshot* pSnapshot );
 
+	CThreadFastMutex									m_FrameSnapshotsWriteMutex;
 	CUtlLinkedList<CFrameSnapshot*, int>		m_FrameSnapshots;
 	CClassMemoryPool< PackedEntity >			m_PackedEntitiesPool;
 

engine/sv_framesnapshot.cpp

@@ -79,10 +79,11 @@ CFrameSnapshot*	CFrameSnapshotManager::NextSnapshot( const CFrameSnapshot *pSnap
 	return m_FrameSnapshots[ next ];
 }
 
-CFrameSnapshot*	CFrameSnapshotManager::CreateEmptySnapshot( int tickcount, int maxEntities )
+CFrameSnapshot*	CFrameSnapshotManager::CreateEmptySnapshot( int tickcount, int maxEntities, uint32 nSnapshotSet )
 {
 	CFrameSnapshot *snap = new CFrameSnapshot;
 	snap->AddReference();
+	snap->m_nSnapshotSet = nSnapshotSet;
 	snap->m_nTickCount = tickcount;
 	snap->m_nNumEntities = maxEntities;
 	snap->m_nValidEntities = 0;
@@ -102,19 +103,21 @@ CFrameSnapshot*	CFrameSnapshotManager::CreateEmptySnapshot( int tickcount, int m
 		entry++;
 	}
 
+	m_FrameSnapshotsWriteMutex.Lock();
 	snap->m_ListIndex = m_FrameSnapshots.AddToTail( snap );
+	m_FrameSnapshotsWriteMutex.Unlock();
 	return snap;
 }
 
 //-----------------------------------------------------------------------------
 // Purpose: 
 // Input  : framenumber - 
 //-----------------------------------------------------------------------------
-CFrameSnapshot* CFrameSnapshotManager::TakeTickSnapshot( int tickcount )
+CFrameSnapshot* CFrameSnapshotManager::TakeTickSnapshot( int tickcount, uint32 nSnapshotSet )
 {
 	unsigned short nValidEntities[MAX_EDICTS];
 
-	CFrameSnapshot *snap = CreateEmptySnapshot( tickcount, sv.num_edicts );
+	CFrameSnapshot *snap = CreateEmptySnapshot( tickcount, sv.num_edicts, nSnapshotSet );
 
 	int maxclients = sv.GetClientCount();
 
@@ -192,7 +195,9 @@ void CFrameSnapshotManager::DeleteFrameSnapshot( CFrameSnapshot* pSnapshot )
 		}
 	}
 
+	m_FrameSnapshotsWriteMutex.Lock();
 	m_FrameSnapshots.Remove( pSnapshot->m_ListIndex );
+	m_FrameSnapshotsWriteMutex.Unlock();
 	delete pSnapshot;
 }
 
@@ -426,7 +431,47 @@ UnpackedDataCache_t *CFrameSnapshotManager::GetCachedUncompressedEntity( PackedE
 	return pdcOldest;
 }
 
+void CFrameSnapshotManager::BuildSnapshotList( CFrameSnapshot *pCurrentSnapshot, CFrameSnapshot *pLastSnapshot, uint32 nSnapshotSet, CReferencedSnapshotList &list )
+{
+	// Keep list building thread-safe
+	m_FrameSnapshotsWriteMutex.Lock();
+
+	int nInsanity = 0;
+	CFrameSnapshot *pSnapshot;
+	if ( pLastSnapshot )
+	{
+		pSnapshot = NextSnapshot( pLastSnapshot );
+	} 
+	else
+	{
+		pSnapshot = pCurrentSnapshot;
+	}
+
+	while ( pSnapshot )
+	{
+		//only add snapshots that match the desired set to the list
+		if( pSnapshot->m_nSnapshotSet == nSnapshotSet )
+		{
+			pSnapshot->AddReference();
+			list.m_vecSnapshots.AddToTail( pSnapshot );
+		}
+
+		++nInsanity;
+		if ( nInsanity > 100000 )
+		{
+			m_FrameSnapshotsWriteMutex.Unlock();
+			Error( "CFrameSnapshotManager::BuildSnapshotList:  infinite loop building list!!!" );
+		}
 
+		if ( pSnapshot == pCurrentSnapshot )
+			break; 
+
+		// got to next snapshot
+		pSnapshot = NextSnapshot( pSnapshot );
+	}
+
+	m_FrameSnapshotsWriteMutex.Unlock();
+}
 
 
 // ------------------------------------------------------------------------------------------------ //

engine/sv_main.cpp

@@ -1788,7 +1788,9 @@ void CGameServer::CopyTempEntities( CFrameSnapshot* pSnapshot )
 //   are running many instances anyway. It's off in Dota and CSGO dedicated servers.
 //
 // Bruce also had a patch to disable this in //ValveGames/staging/game/tf/cfg/unencrypted/print_instance_config.py
-static ConVar sv_parallel_sendsnapshot( "sv_parallel_sendsnapshot", "0" );
+//
+// NOTE: The crash should be fixed.
+static ConVar sv_parallel_sendsnapshot( "sv_parallel_sendsnapshot", "1" );
 
 static void SV_ParallelSendSnapshot( CGameClient *& pClient )
 {