Fix issues with refcounting of reliable segments.

If our ping is very high and send rate is low, we can easily receive ACKs
for all outstanding reliable data of a message we have sent, while it is
still the first item in the queue and we still have more to send for that
message.  If this happens, we would decrement the last reference count and
delete the msssage, even though we haven't finished sending it.  To fix
this, just have the queue hold a reference to the message.

P4:6868998
This commit is contained in:
Fletcher Dunn
2021-11-01 18:10:18 -07:00
parent dfdc47378f
commit 9f81579f20
2 changed files with 21 additions and 7 deletions
@@ -216,6 +216,10 @@ void SSNPSenderState::RemoveRefCountReliableSegment( uint16 hSeg )
if ( --info.m_nSentReliableSegRefCount <= 0 )
{
DbgAssert( info.m_nSentReliableSegRefCount == 0 );
// We should not be in the lane send queue
Assert( pMsg->m_linksSecondaryQueue.m_pQueue == nullptr );
pMsg->Unlink();
pMsg->Release();
}
@@ -357,7 +361,7 @@ int64 CSteamNetworkConnectionBase::SNP_SendMessage( CSteamNetworkingMessage *pSe
hdrEnd = SerializeVarInt( hdrEnd, cbData>>5U );
}
reliableInfo.m_cbHdr = hdrEnd - hdr;
reliableInfo.m_nSentReliableSegRefCount = 0;
reliableInfo.m_nSentReliableSegRefCount = 1; // Initialize reference count to 1.
// Grow the total size of the message by the header
pSendMessage->m_cbSize += reliableInfo.m_cbHdr;
@@ -2514,6 +2518,7 @@ int CSteamNetworkConnectionBase::SNP_SerializePacketInternal( SNPPacketSerialize
// sure that we don't make an excessively large
// one and then have a hard time retrying it later.
int cbDesiredSegSize = pSendMsg->m_cbSize - sendLane.m_cbCurrentSendMessageSent;
Assert( cbDesiredSegSize > 0 );
if ( cbDesiredSegSize > m_cbMaxReliableMessageSegment )
{
cbDesiredSegSize = m_cbMaxReliableMessageSegment;
@@ -2635,6 +2640,15 @@ int CSteamNetworkConnectionBase::SNP_SerializePacketInternal( SNPPacketSerialize
if ( !k_bUnreliableOnly && pSendMsg->SNPSend_IsReliable() )
{
// We hold a reference while in the lane send queue. But
// we've been removed, so decrement that reference count now.
// Note that this might drop our reference count to zero.
// But there should be at least one segment ready to be serialized
// which will hold a reference to us. We just haven't added it yet.
CSteamNetworkingMessage::ReliableSendInfo_t &relInfo = pSendMsg->ReliableSendInfo();
Assert( relInfo.m_nSentReliableSegRefCount > 0 );
--relInfo.m_nSentReliableSegRefCount;
// Go ahead and add us to the end of the list of unacked messages
pSeg->m_pMsg->LinkToQueueTail( &CSteamNetworkingMessage::m_links, &m_senderState.m_unackedReliableMessages );
}
@@ -128,10 +128,12 @@ public:
inline void SNPSend_SetReliableStreamPos( int64 x ) { Assert( m_nFlags & k_nSteamNetworkingSend_Reliable ); ReliableSendInfo().m_nStreamPos = x; }
// Working data for reliable messages.
// !KLUDGE! Reuse the identity field
struct ReliableSendInfo_t
{
int64 m_nStreamPos;
// Number of reliable segments that refer to this message.
// Also while we are in the queue waiting to be sent the queue holds a reference
int m_nSentReliableSegRefCount;
int m_cbHdr;
byte m_hdr[16];
@@ -139,6 +141,8 @@ public:
const ReliableSendInfo_t &ReliableSendInfo() const
{
DbgAssert( m_nFlags & k_nSteamNetworkingSend_Reliable );
// !KLUDGE! Reuse the identity field. We don't actually put this in a union because
// this is internal stuff that doesn't need to be exposed in the API
COMPILE_TIME_ASSERT( sizeof(m_identityPeer) >= sizeof(ReliableSendInfo_t) );
return *(ReliableSendInfo_t *)&m_identityPeer;
}
@@ -412,11 +416,7 @@ struct SSNPSenderState
SteamNetworkingMessageQueue m_messagesQueued;
/// List of reliable messages that have been fully placed on the wire at least once,
/// but we're hanging onto because of the potential need to retry. (Note that if we get
/// packet loss, it's possible that we hang onto a message even after it's been fully
/// acked, because a prior message is still needed. We always operate on this list
/// like a queue, rather than seeking into the middle of the list and removing messages
/// as soon as they are no longer needed.)
/// but we're hanging onto because of the potential need to retry.
SteamNetworkingMessageQueue m_unackedReliableMessages;
// Buffered data counters. See SteamNetworkingQuickConnectionStatus for more info