From e5e8052485c6445d343b777f969bcfe7ee78eeb9 Mon Sep 17 00:00:00 2001 From: Fletcher Dunn Date: Tue, 6 Jul 2021 15:13:40 -0700 Subject: [PATCH] Fix P2P scheduling bug related to processing timeouts and anti-flap checking. CSteamNetworkConnectionP2P was scheduleing a wakeup call EXACTLY at the same time as the timeout. But it was not actually responsible for processing the timeout, the transport does that. The transport did properly have a method scheduled to deal with it -- but they were both using the same wake up time. So the connection kept waking up first, and scheduling the same wake up time as the transport, and then winning the tie, repeatedly. The fix is to make sure that we wake up *just* after the transport has dealt with the timeout. It's possible that the thinker system could deal with these sorts of cycles better, but I'd rather avoid adding that complication until I find a problem that doesn't have a simple solution like this. P4:6653586 --- .../clientlib/steamnetworkingsockets_p2p.cpp | 7 ++++++- .../clientlib/steamnetworkingsockets_p2p.h | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_p2p.cpp b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_p2p.cpp index f4f7616..fbc5a6b 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_p2p.cpp +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_p2p.cpp @@ -1417,7 +1417,12 @@ void CSteamNetworkConnectionP2P::ThinkSelectTransport( SteamNetworkingMicrosecon } \ else if ( pTransport->m_usecEndToEndInFlightReplyTimeout > 0 ) \ { \ - m_usecNextEvaluateTransport = std::min( m_usecNextEvaluateTransport, pTransport->m_usecEndToEndInFlightReplyTimeout ); \ + /* The transport *should* be scheduled to deal with the timeout */ \ + Assert( pTransport->GetP2PTransportThinkScheduleTime() <= pTransport->m_usecEndToEndInFlightReplyTimeout ); \ + /* In case not, use the max so we don't wake up and try to deal with it before it has taken care of it and get into a loop */ \ + SteamNetworkingMicroseconds usecAfterTimeOut = std::max( pTransport->GetP2PTransportThinkScheduleTime(), pTransport->m_usecEndToEndInFlightReplyTimeout ); \ + ++usecAfterTimeOut; /* because we need to make sure we do our processing AFTER the transport has had a chance to deal with the timeout */ \ + m_usecNextEvaluateTransport = std::min( m_usecNextEvaluateTransport, usecAfterTimeOut ); \ } \ else \ { \ diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_p2p.h b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_p2p.h index acdb8a1..707814c 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_p2p.h +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_p2p.h @@ -163,6 +163,8 @@ public: bool TryLock() { return m_pSelfAsConnectionTransport->m_connection.TryLock(); } void Unlock() const { return m_pSelfAsConnectionTransport->m_connection.Unlock(); } + inline SteamNetworkingMicroseconds GetP2PTransportThinkScheduleTime() const { return m_scheduleP2PTransportThink.GetScheduleTime(); } + protected: CConnectionTransportP2PBase( const char *pszDebugName, CConnectionTransport *pSelfBase ); virtual ~CConnectionTransportP2PBase();