GitHub/GameNetworkingSockets
1
0
Fork 0
mirror of https://github.com/ValveSoftware/GameNetworkingSockets.git synced 2026-05-29 16:20:34 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix a subtle bug if ping increases suddenly

Browse Source 
If latency spikes very suddenly and dramatically, we won't ever update our
estimate.

The problem is that we use the RTT to decide when we can forget about old
packets that we have marked as NACK because they timed out.  But this expiry
is based on our RTT estimate, which in this case is too small, perhaps
significantly too small.  And then, when those packets do eventually get
acked, we cannot update our RTT estimate, because we have forgotten about the
packet being acked!  So we will be stuck with an RTT estimate that is
permanently inaccurate.

The clue that something was wrong was in the connection test, I should
have noticed it.  The ping time goes from 0 (it's loopback) to 200ms
(because we are faking it).  And the ping time estimates being displayed
were still ~1ms, even though we were faking 200ms.

This is probably extremely rare in practice, but still it is relatively
catastrophic when it happens, so it's good to fix it.

P4:6402130
This commit is contained in:
Fletcher Dunn
2021-03-05 14:31:21 -08:00
parent 8b823430d2
commit ee6ed901bf
1 changed files with 20 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp
+20 -1
View File
@@ -1211,7 +1211,26 @@ SteamNetworkingMicroseconds CSteamNetworkConnectionBase::SNP_SenderCheckInFlight
++inFlightPkt;
// Expire old packets (all of these should have been marked as nacked)
SteamNetworkingMicroseconds usecWhenExpiry = usecNow - usecRTO*2;
// Here we need to be careful when selecting an expiry. If the actual RTT
// time suddenly increases, using the current RTT estimate may expire them
// too quickly. This can actually be catastrophic, since if we forgot
// about these packets now, and then they later are acked, we won't be able
// to update our RTT, and so we will be stuck with an RTT estimate that
// is too small
SteamNetworkingMicroseconds usecExpiry = usecRTO*2;
if ( m_statsEndToEnd.m_ping.m_nValidPings < 1 )
{
usecExpiry += k_nMillion;
}
else
{
SteamNetworkingMicroseconds usecMostRecentPingAge = usecNow - m_statsEndToEnd.m_ping.TimeRecvMostRecentPing();
usecMostRecentPingAge = std::min( usecMostRecentPingAge, k_nMillion*3 );
if ( usecMostRecentPingAge > usecExpiry )
usecExpiry = usecMostRecentPingAge;
}
SteamNetworkingMicroseconds usecWhenExpiry = usecNow - usecExpiry;
for (;;)
{
if ( inFlightPkt->second.m_usecWhenSent > usecWhenExpiry )