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 memory leak.

Browse Source 
Two changes to cleanup.  First, to it earlier, as soon as the connection
switches into a state where we know we will not need to send or receive
messages anymore.  (Don't wait until object destruction.)  Second, we
were not freeing up any queued messages in the m_messagesQueued linked
list.

Also fix a monir bug in the linger functionality.  The connection is still
"connected" for all internal purposes, it just is not visible to the API
anymore.  So we need our stats object to stay alive, and we expect our peer
to be replying in a timely manner.

Add some asserts and paranoia.
This commit is contained in:
Fletcher Dunn
2019-05-06 09:14:14 -07:00
parent ec286242b6
commit f932e64ebc
4 changed files with 100 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp
+16 -7
View File
@@ -1482,7 +1482,7 @@ void CSteamNetworkConnectionBase::ConnectionStateChanged( ESteamNetworkingConnec
if ( eNewAPIState == k_ESteamNetworkingConnectionState_None )
m_queueRecvMessages.PurgeMessages();
// Check crypto state
// Slam some stuff when we are in various states
switch ( GetState() )
{
case k_ESteamNetworkingConnectionState_Dead:
@@ -1497,19 +1497,28 @@ void CSteamNetworkConnectionBase::ConnectionStateChanged( ESteamNetworkingConnec
// And let stats tracking system know that it shouldn't
// expect to be able to get stuff acked, etc
m_statsEndToEnd.SetDisconnected( true, m_usecWhenEnteredConnectionState );
// Go head and free up memory now
SNP_ShutdownConnection();
break;
case k_ESteamNetworkingConnectionState_Linger:
// Don't bother trading stats back and forth with peer,
// the only message we will send to them is "connection has been closed"
m_statsEndToEnd.SetDisconnected( true, m_usecWhenEnteredConnectionState );
break;
case k_ESteamNetworkingConnectionState_Connected:
case k_ESteamNetworkingConnectionState_FindingRoute:
// Key exchange should be complete
Assert( m_bCryptKeysValid );
// Link stats tracker should send and expect, acks, keepalives, etc
m_statsEndToEnd.SetDisconnected( false, m_usecWhenEnteredConnectionState );
break;
case k_ESteamNetworkingConnectionState_FindingRoute:
// Key exchange should be complete. (We do that when accepting a connection.)
Assert( m_bCryptKeysValid );
// FIXME. Probably we should NOT set the stats tracker as "connected" yet.
//Assert( m_statsEndToEnd.IsDisconnected() );
m_statsEndToEnd.SetDisconnected( false, m_usecWhenEnteredConnectionState );
break;
src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h
+1
View File
@@ -623,6 +623,7 @@ protected:
//
void SNP_InitializeConnection( SteamNetworkingMicroseconds usecNow );
void SNP_ShutdownConnection();
EResult SNP_SendMessage( SteamNetworkingMicroseconds usecNow, const void *pData, int cbData, int nSendFlags );
SteamNetworkingMicroseconds SNP_ThinkSendState( SteamNetworkingMicroseconds usecNow );
SteamNetworkingMicroseconds SNP_GetNextThinkTime( SteamNetworkingMicroseconds usecNow );
src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp
+63 -29
View File
@@ -142,12 +142,16 @@ inline SteamNetworkingMicroseconds GetUsecPingWithFallback( CSteamNetworkConnect
return nPingMS*1000;
}
void SSNPSenderState::Reset()
void SSNPSenderState::Shutdown()
{
while ( !m_unackedReliableMessages.empty() )
{
delete m_unackedReliableMessages.pop_front();
}
m_unackedReliableMessages.delete_all();
m_messagesQueued.delete_all();
m_mapInFlightPacketsByPktNum.clear();
m_listInFlightReliableRange.clear();
m_cbPendingUnreliable = 0;
m_cbPendingReliable = 0;
m_cbSentUnackedReliable = 0;
}
//-----------------------------------------------------------------------------
@@ -213,7 +217,6 @@ SSNPSenderState::SSNPSenderState()
SSNPReceiverState::SSNPReceiverState()
{
// Init packet gaps with a sentinel
m_mapPacketGaps.clear();
SSNPPacketGap &sentinel = m_mapPacketGaps[INT64_MAX];
sentinel.m_nEnd = INT64_MAX; // Fixed value
sentinel.m_usecWhenOKToNack = INT64_MAX; // Fixed value, for when there is nothing left to nack
@@ -225,6 +228,15 @@ SSNPReceiverState::SSNPReceiverState()
m_itPendingNack = m_itPendingAck;
}
//-----------------------------------------------------------------------------
void SSNPReceiverState::Shutdown()
{
m_mapUnreliableSegments.clear();
m_bufReliableStream.clear();
m_mapReliableStreamGaps.clear();
m_mapPacketGaps.clear();
}
//-----------------------------------------------------------------------------
void CSteamNetworkConnectionBase::SNP_InitializeConnection( SteamNetworkingMicroseconds usecNow )
{
@@ -248,6 +260,13 @@ void CSteamNetworkConnectionBase::SNP_InitializeConnection( SteamNetworkingMicro
SNP_ClampSendRate();
}
//-----------------------------------------------------------------------------
void CSteamNetworkConnectionBase::SNP_ShutdownConnection()
{
m_senderState.Shutdown();
m_receiverState.Shutdown();
}
//-----------------------------------------------------------------------------
EResult CSteamNetworkConnectionBase::SNP_SendMessage( SteamNetworkingMicroseconds usecNow, const void *pData, int cbData, int nSendFlags )
{
@@ -3071,6 +3090,13 @@ void SSNPReceiverState::DebugCheckPackGapMap() const
SteamNetworkingMicroseconds CSteamNetworkConnectionBase::SNP_TimeWhenWantToSendNextPacket() const
{
// We really shouldn't be trying to do this when not connected
if ( !BStateIsConnectedForWirePurposes() )
{
AssertMsg( false, "We shouldn't be trying to send packets when not fully connected" );
return k_nThinkTime_Never;
}
// When does the sender want to send data?
SteamNetworkingMicroseconds usecNextSend = m_senderState.TimeWhenWantToSendNextPacket();
@@ -3146,33 +3172,41 @@ void CSteamNetworkConnectionBase::SNP_PopulateQuickStats( SteamNetworkingQuickCo
info.m_cbPendingUnreliable = m_senderState.m_cbPendingUnreliable;
info.m_cbPendingReliable = m_senderState.m_cbPendingReliable;
info.m_cbSentUnackedReliable = m_senderState.m_cbSentUnackedReliable;
// Accumulate tokens so that we can properly predict when the next time we'll be able to send something is
SNP_TokenBucket_Accumulate( usecNow );
//
// Time until we can send the next packet
// If anything is already queued, then that will have to go out first. Round it down
// to the nearest packet.
//
// NOTE: This ignores the precise details of SNP framing. If there are tons of
// small packets, it'll actually be worse. We might be able to approximate that
// the framing overhead better by also counting up the number of *messages* pending.
// Probably not worth it here, but if we had that number available, we'd use it.
int cbPendingTotal = m_senderState.PendingBytesTotal() / k_cbSteamNetworkingSocketsMaxMessageNoFragment * k_cbSteamNetworkingSocketsMaxMessageNoFragment;
// Adjust based on how many tokens we have to spend now (or if we are already
// over-budget and have to wait until we could spend another)
cbPendingTotal -= (int)m_senderState.m_flTokenBucket;
if ( cbPendingTotal <= 0 )
if ( GetState() == k_ESteamNetworkingConnectionState_Connected )
{
// We could send it right now.
info.m_usecQueueTime = 0;
// Accumulate tokens so that we can properly predict when the next time we'll be able to send something is
SNP_TokenBucket_Accumulate( usecNow );
//
// Time until we can send the next packet
// If anything is already queued, then that will have to go out first. Round it down
// to the nearest packet.
//
// NOTE: This ignores the precise details of SNP framing. If there are tons of
// small packets, it'll actually be worse. We might be able to approximate that
// the framing overhead better by also counting up the number of *messages* pending.
// Probably not worth it here, but if we had that number available, we'd use it.
int cbPendingTotal = m_senderState.PendingBytesTotal() / k_cbSteamNetworkingSocketsMaxMessageNoFragment * k_cbSteamNetworkingSocketsMaxMessageNoFragment;
// Adjust based on how many tokens we have to spend now (or if we are already
// over-budget and have to wait until we could spend another)
cbPendingTotal -= (int)m_senderState.m_flTokenBucket;
if ( cbPendingTotal <= 0 )
{
// We could send it right now.
info.m_usecQueueTime = 0;
}
else
{
info.m_usecQueueTime = (int64)cbPendingTotal * k_nMillion / SNP_ClampSendRate();
}
}
else
{
info.m_usecQueueTime = (int64)cbPendingTotal * k_nMillion / SNP_ClampSendRate();
// We'll never be able to send it. (Or, we don't know when that will be.)
info.m_usecQueueTime = INT64_MAX;
}
}
src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.h
+20 -2
View File
@@ -119,6 +119,14 @@ struct SSNPSendMessageList
return false;
}
/// Delete all elements. This should only be called if you own the messages!
void delete_all()
{
while ( m_pFirst )
delete pop_front();
Assert( m_pLast == nullptr );
}
/// Unlink the message at the head, if any and return it.
/// Unlike STL pop_front, this will return nullptr if the
/// list is empty
@@ -172,9 +180,9 @@ struct SSNPSenderState
{
SSNPSenderState();
~SSNPSenderState() {
Reset();
Shutdown();
}
void Reset();
void Shutdown();
/// Current sending rate in bytes per second, RFC 3448 4.2 states default
/// is one packet per second, but that is insane and we're not doing that.
@@ -352,6 +360,10 @@ struct SSNPPacketGap
struct SSNPReceiverState
{
SSNPReceiverState();
~SSNPReceiverState() {
Shutdown();
}
void Shutdown();
/// Unreliable message segments that we have received. When an unreliable message
/// needs to be fragmented, we store the pieces here. NOTE: it might be more efficient
@@ -444,6 +456,12 @@ struct SSNPReceiverState
/// if we don't have any acks pending right now.
inline SteamNetworkingMicroseconds TimeWhenFlushAcks() const
{
// Paranoia
if ( m_mapPacketGaps.empty() )
{
AssertMsg( false, "TimeWhenFlushAcks - we're shut down!" );
return INT64_MAX;
}
return m_itPendingAck->second.m_usecWhenAckPrior;
}