SNP: fix stop-waiting clamp and improve sentinel guard in gap-trim loop

Fixed a latent bug where nPktNumBeforeSentinelGap was declared int instead
of int64. On connections that have exchanged more than ~2.1 billion
packets, the sentinel key truncates to a large negative value, the clamp
misfires unconditionally, and m_nMinPktNumToSendAcks gets set to that
negative value -- permanently breaking stop-waiting advancement for that
connection.

Also rewrote the paranoia guard that protects the gap-trim loop from walking
into the sentinel. The old check was buried inside the m_nEnd > nMinPktNumToSendAcks
branch (which the sentinel always entered) and contained a dead
Assert( h == end() ) that could never be true. The new check uses an explicit
itSentinel iterator captured before the loop, tests for it first before
consulting m_nEnd, and adds a belt-and-suspenders m_nEnd == INT64_MAX condition.
Changed AssertMsg to AssertMsgOnce since this is a paranoia path that should
never fire in practice.

Author: zpostfacto

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp

@@ -1014,7 +1014,7 @@ bool CSteamNetworkConnectionBase::ProcessPlainTextDataChunk( int usecTimeSinceLa
 			// Clamping the requested stop waiting point to the highest packet number we
 			// received keeps a lot of code simple.  If this clamp activates, the gap map
 			// will get totally emptied by the loop below.
-			const int nPktNumBeforeSentinelGap = m_receiverState.m_mapPacketGaps.rbegin()->first-1;
+			const int64 nPktNumBeforeSentinelGap = m_receiverState.m_mapPacketGaps.rbegin()->first-1;
 			Assert( nPktNumBeforeSentinelGap <= m_statsEndToEnd.m_nMaxRecvPktNum );
 			if ( unlikely( nMinPktNumToSendAcks > nPktNumBeforeSentinelGap ) )
 			{
@@ -1047,20 +1047,21 @@ bool CSteamNetworkConnectionBase::ProcessPlainTextDataChunk( int usecTimeSinceLa
 			// Trim from the front of the packet gap list,
 			// we can stop reporting these losses to the sender
 			auto h = m_receiverState.m_mapPacketGaps.begin();
+			const auto itSentinel = std::prev( m_receiverState.m_mapPacketGaps.end() );
 			while ( h->first <= m_receiverState.m_nMinPktNumToSendAcks )
 			{
 				Assert( h->first < h->second.m_nEnd );
-				if ( h->second.m_nEnd > m_receiverState.m_nMinPktNumToSendAcks )
+
+				// We should never reach the sentinel, due to the clamp above.
+				// But add a little paranoia check here and don't crash, just in case.
+				if ( unlikely( h == itSentinel || h->second.m_nEnd == INT64_MAX ) )
 				{
+					AssertMsgOnce( false, "SNP stop waiting advanced past sentinel gap.  This should never happen!" );
+					break;
+				}
 
-					// We should never reach the sentinel, due to the clamp above.
-					// But we will add a little paranoia check here, just in case.
-					if ( unlikely( h->first > nPktNumBeforeSentinelGap ) )
-					{
-						Assert( h == m_receiverState.m_mapPacketGaps.end() );
-						AssertMsg( false, "SNP stop waiting advanced past sentinel gap.  This should never happen!" );
-						break;
-					}
+				if ( h->second.m_nEnd > m_receiverState.m_nMinPktNumToSendAcks )
+				{
 
 					// Ug.  You're not supposed to modify the key in a map.
 					// I suppose that's legit, since you could violate the ordering.
@@ -3759,7 +3760,7 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int
 				// would be to make it more clear what happened.  Either way, the connection
 				// is dead at this point if we get here because of protobuf encoding having
 				// too many continuation bytes.)
-				// 
+				//
 				// Return true here because the packet containing this segment is OK and can be acked.
 				return true;
 			}
@@ -3803,7 +3804,7 @@ bool CSteamNetworkConnectionBase::SNP_ReceiveReliableSegment( int64 nPktNum, int
 			{
 				// We haven't received enough of the message to decode the size
 				// (Probably.  See note above about the possibility of bogus protobuf data.)
-				// 
+				//
 				// Return true here because the packet containing this segment is OK and can be acked.
 				return true;
 			}