From f7917fdf5629c549e47eec85dfcf2735dff220bb Mon Sep 17 00:00:00 2001 From: Fletcher Dunn Date: Sun, 12 Apr 2020 13:13:09 -0700 Subject: [PATCH] A fixes for pipe connections. - Use m_bSupressStateChangeCallbacks instead of overridding callbacks and checking for special cases - Don't call destructors directly, use ConnectionDestroySelfNow() - Fix a few other little things --- .../steamnetworkingsockets_connections.cpp | 42 ++++++++++--------- .../steamnetworkingsockets_connections.h | 4 +- .../clientlib/steamnetworkingsockets_udp.cpp | 23 +++++----- .../clientlib/steamnetworkingsockets_udp.h | 1 - 4 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp index ad56b10..ba92237 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp @@ -2872,27 +2872,31 @@ bool CSteamNetworkConnectionPipe::APICreateSocketPair( CSteamNetworkingSockets * if ( !pConn[0] || !pConn[1] ) { failed: - delete pConn[0]; pConn[0] = nullptr; - delete pConn[1]; pConn[1] = nullptr; + pConn[0]->ConnectionDestroySelfNow(); pConn[0] = nullptr; + pConn[1]->ConnectionDestroySelfNow(); pConn[1] = nullptr; return false; } pConn[0]->m_pPartner = pConn[1]; pConn[1]->m_pPartner = pConn[0]; + // Don't post any state changes for these transitions. We just want to immediately start in the + // connected state + pConn[0]->m_bSupressStateChangeCallbacks = true; + pConn[1]->m_bSupressStateChangeCallbacks = true; + // Do generic base class initialization for ( int i = 0 ; i < 2 ; ++i ) { - if ( !pConn[i]->BInitConnection( usecNow, 0, nullptr, errMsg ) ) + CSteamNetworkConnectionPipe *p = pConn[i]; + CSteamNetworkConnectionPipe *q = pConn[1-i]; + if ( !p->BInitConnection( usecNow, 0, nullptr, errMsg ) ) { AssertMsg1( false, "CSteamNetworkConnectionPipe::BInitConnection failed. %s", errMsg ); goto failed; } - - // Slam in a really large SNP rate - int nRate = 0x10000000; - pConn[i]->m_connectionConfig.m_SendRateMin.Set( nRate ); - pConn[i]->m_connectionConfig.m_SendRateMax.Set( nRate ); + p->m_identityRemote = q->m_identityLocal; + p->m_unConnectionIDRemote = q->m_unConnectionIDLocal; } // Exchange some dummy "connect" packets so that all of our internal variables @@ -2915,6 +2919,9 @@ failed: p->ConnectionState_Connected( usecNow ); } + // Any further state changes are legit + pConn[0]->m_bSupressStateChangeCallbacks = false; + pConn[1]->m_bSupressStateChangeCallbacks = false; return true; } @@ -2928,6 +2935,11 @@ CSteamNetworkConnectionPipe::CSteamNetworkConnectionPipe( CSteamNetworkingSocket // This is not strictly necessary, but it's nice to make it official m_connectionConfig.m_Unencrypted.Set( 3 ); + + // Slam in a really large SNP rate so that we are never rate limited + int nRate = 0x10000000; + m_connectionConfig.m_SendRateMin.Set( nRate ); + m_connectionConfig.m_SendRateMax.Set( nRate ); } CSteamNetworkConnectionPipe::~CSteamNetworkConnectionPipe() @@ -3032,8 +3044,8 @@ void CSteamNetworkConnectionPipe::SendEndToEndStatsMsg( EStatsReplyRequest eRequ m_pPartner->FakeSendStats( usecNow, 0 ); // ... and us receiving it immediately - m_pPartner->m_statsEndToEnd.PeerAckedLifetime( usecNow ); - m_pPartner->m_statsEndToEnd.PeerAckedInstantaneous( usecNow ); + m_statsEndToEnd.PeerAckedLifetime( usecNow ); + m_statsEndToEnd.PeerAckedInstantaneous( usecNow ); } bool CSteamNetworkConnectionPipe::BCanSendEndToEndConnectRequest() const @@ -3108,16 +3120,6 @@ void CSteamNetworkConnectionPipe::ConnectionStateChanged( ESteamNetworkingConnec } } -void CSteamNetworkConnectionPipe::PostConnectionStateChangedCallback( ESteamNetworkingConnectionState eOldAPIState, ESteamNetworkingConnectionState eNewAPIState ) -{ - // Don't post any callbacks for the initial transitions. - if ( eNewAPIState == k_ESteamNetworkingConnectionState_Connecting || eNewAPIState == k_ESteamNetworkingConnectionState_Connected ) - return; - - // But post callbacks for these guys - CSteamNetworkConnectionBase::PostConnectionStateChangedCallback( eOldAPIState, eNewAPIState ); -} - void CSteamNetworkConnectionPipe::DestroyTransport() { // Using the same object for connection and transport diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h index 6ddb32e..bed9453 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h @@ -747,11 +747,12 @@ protected: /// Dummy loopback/pipe connection that doesn't actually do any network work. /// For these types of connections, the distinction between connection and transport -/// is not realyl useful +/// is not really useful class CSteamNetworkConnectionPipe final : public CSteamNetworkConnectionBase, public CConnectionTransport { public: + /// Create a pair of loopback connections that are immediately connected to each other static bool APICreateSocketPair( CSteamNetworkingSockets *pSteamNetworkingSocketsInterface, CSteamNetworkConnectionPipe **pOutConnections, const SteamNetworkingIdentity pIdentity[2] ); /// The guy who is on the other end. @@ -759,7 +760,6 @@ public: // CSteamNetworkConnectionBase overrides virtual int64 _APISendMessageToConnection( CSteamNetworkingMessage *pMsg, SteamNetworkingMicroseconds usecNow, bool *pbThinkImmediately ) override; - virtual void PostConnectionStateChangedCallback( ESteamNetworkingConnectionState eOldAPIState, ESteamNetworkingConnectionState eNewAPIState ) override; virtual void InitConnectionCrypto( SteamNetworkingMicroseconds usecNow ) override; virtual EUnsignedCert AllowRemoteUnsignedCert() override; virtual EUnsignedCert AllowLocalUnsignedCert() override; diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_udp.cpp b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_udp.cpp index 1197b7c..11ebdb2 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_udp.cpp +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_udp.cpp @@ -1823,16 +1823,6 @@ EUnsignedCert CSteamNetworkConnectionlocalhostLoopback::AllowLocalUnsignedCert() return k_EUnsignedCert_Allow; } -void CSteamNetworkConnectionlocalhostLoopback::PostConnectionStateChangedCallback( ESteamNetworkingConnectionState eOldAPIState, ESteamNetworkingConnectionState eNewAPIState ) -{ - // Don't post any callbacks for the initial transitions. - if ( eNewAPIState == k_ESteamNetworkingConnectionState_Connecting || eNewAPIState == k_ESteamNetworkingConnectionState_Connected ) - return; - - // But post callbacks for these guys - CSteamNetworkConnectionUDP::PostConnectionStateChangedCallback( eOldAPIState, eNewAPIState ); -} - bool CSteamNetworkConnectionlocalhostLoopback::APICreateSocketPair( CSteamNetworkingSockets *pSteamNetworkingSocketsInterface, CSteamNetworkConnectionlocalhostLoopback *pConn[2], const SteamNetworkingIdentity pIdentity[2] ) { SteamDatagramTransportLock::AssertHeldByCurrentThread(); @@ -1844,11 +1834,16 @@ bool CSteamNetworkConnectionlocalhostLoopback::APICreateSocketPair( CSteamNetwor if ( !pConn[0] || !pConn[1] ) { failed: - delete pConn[0]; pConn[0] = nullptr; - delete pConn[1]; pConn[1] = nullptr; + pConn[0]->ConnectionDestroySelfNow(); pConn[0] = nullptr; + pConn[1]->ConnectionDestroySelfNow(); pConn[1] = nullptr; return false; } + // Don't post any state changes for these transitions. We just want to immediately start in the + // connected state + pConn[0]->m_bSupressStateChangeCallbacks = true; + pConn[1]->m_bSupressStateChangeCallbacks = true; + CConnectionTransportUDP *pTransport[2] = { new CConnectionTransportUDP( *pConn[0] ), new CConnectionTransportUDP( *pConn[1] ) @@ -1887,6 +1882,10 @@ failed: p->ConnectionState_Connected( usecNow ); } + // Any further state changes are legit + pConn[0]->m_bSupressStateChangeCallbacks = false; + pConn[1]->m_bSupressStateChangeCallbacks = false; + return true; } diff --git a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_udp.h b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_udp.h index bd7c316..a9282f9 100644 --- a/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_udp.h +++ b/src/steamnetworkingsockets/clientlib/steamnetworkingsockets_udp.h @@ -159,7 +159,6 @@ public: static bool APICreateSocketPair( CSteamNetworkingSockets *pSteamNetworkingSocketsInterface, CSteamNetworkConnectionlocalhostLoopback *pConn[2], const SteamNetworkingIdentity pIdentity[2] ); /// Base class overrides - virtual void PostConnectionStateChangedCallback( ESteamNetworkingConnectionState eOldAPIState, ESteamNetworkingConnectionState eNewAPIState ) override; virtual EUnsignedCert AllowRemoteUnsignedCert() override; virtual EUnsignedCert AllowLocalUnsignedCert() override; };