Skip to content

Commit 3b03992

Browse files
committed
Refactor responsibility of releasing failed messages
APISendMessageToConnection, _APISendMessageToConnection, and SNP_SendMessage now expect the *caller* to release the message if it changes. Since there are many failure points but only a few call sites, this resulted in a net reduction in lines of code and is imo simpler. But more importantly it sets us up so that we can continue this one level higher, into the public API, so that we can allow the app to retain failed messages. For lots of app code, I anticipate that this will be useful so that they can retry them later if they want. The most important and legitimate failure case here is if the send buffer is full.
1 parent 75451a0 commit 3b03992

5 files changed

Lines changed: 6 additions & 12 deletions

File tree

src/steamnetworkingsockets/clientlib/csteamnetworkingmessages.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,7 @@ EResult CSteamNetworkingMessages::SendMessageToUser( const SteamNetworkingIdenti
518518
int64 nMsgNumberOrResult = pConn->_APISendMessageToConnection( pMsg, usecNow, nullptr );
519519
if ( nMsgNumberOrResult > 0 )
520520
return k_EResultOK;
521+
pMsg->Release();
521522
return EResult( -nMsgNumberOrResult );
522523
}
523524

src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,6 +1364,8 @@ void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessag
13641364
result = pConn->APISendMessageToConnection( pMsg, usecNow, &bThinkImmediately );
13651365
if ( bThinkImmediately )
13661366
bConnectionThinkImmediately = true;
1367+
if ( result <= 0 )
1368+
pMsg->Release();
13671369
}
13681370
else
13691371
{

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2053,6 +2053,7 @@ EResult CSteamNetworkConnectionBase::APISendMessageToConnection( const void *pDa
20532053
*pOutMessageNumber = nMsgNumberOrResult;
20542054
return k_EResultOK;
20552055
}
2056+
pMsg->Release();
20562057
return EResult( -nMsgNumberOrResult );
20572058
}
20582059

@@ -2069,24 +2070,19 @@ int64 CSteamNetworkConnectionBase::APISendMessageToConnection( CSteamNetworkingM
20692070
case k_ESteamNetworkingConnectionState_Dead:
20702071
default:
20712072
AssertMsg( false, "Why are making API calls on this connection?" );
2072-
pMsg->Release();
20732073
return -k_EResultInvalidState;
20742074

20752075
case k_ESteamNetworkingConnectionState_Connecting:
20762076
case k_ESteamNetworkingConnectionState_FindingRoute:
20772077
if ( pMsg->m_nFlags & k_nSteamNetworkingSend_NoDelay )
2078-
{
2079-
pMsg->Release();
20802078
return -k_EResultIgnored;
2081-
}
20822079
break;
20832080

20842081
case k_ESteamNetworkingConnectionState_Connected:
20852082
break;
20862083

20872084
case k_ESteamNetworkingConnectionState_ClosedByPeer:
20882085
case k_ESteamNetworkingConnectionState_ProblemDetectedLocally:
2089-
pMsg->Release();
20902086
return -k_EResultNoConnection;
20912087
}
20922088

@@ -2095,7 +2091,6 @@ int64 CSteamNetworkConnectionBase::APISendMessageToConnection( CSteamNetworkingM
20952091
if ( (unsigned)pMsg->m_cbSize > (unsigned)cbMaxMessageSizeSend )
20962092
{
20972093
SpewWarning( "Message size %u is too big. Max is %d", pMsg->m_cbSize, cbMaxMessageSizeSend );
2098-
pMsg->Release();
20992094
return -k_EResultInvalidParam;
21002095
}
21012096

@@ -2112,7 +2107,6 @@ int64 CSteamNetworkConnectionBase::_APISendMessageToConnection( CSteamNetworking
21122107
if ( (unsigned)pMsg->m_cbSize > (unsigned)k_cbMaxSteamNetworkingSocketsMessageSizeSend_Internal )
21132108
{
21142109
AssertMsg2( false, "Message size %d is too big. Max internal size is %d", pMsg->m_cbSize, k_cbMaxSteamNetworkingSocketsMessageSizeSend_Internal );
2115-
pMsg->Release();
21162110
return -k_EResultInvalidParam;
21172111
}
21182112

@@ -4069,13 +4063,11 @@ int64 CSteamNetworkConnectionPipe::_APISendMessageToConnection( CSteamNetworking
40694063
{
40704064
// Caller should have checked the connection at a higher level, so this is a bug
40714065
AssertMsg( false, "No partner pipe?" );
4072-
pMsg->Release();
40734066
return -k_EResultFail;
40744067
}
40754068

40764069
if ( (int)pMsg->m_idxLane >= len( m_senderState.m_vecLanes ) )
40774070
{
4078-
pMsg->Release();
40794071
return -k_EResultInvalidParam;
40804072
}
40814073
SSNPSenderState::Lane &lane = m_senderState.m_vecLanes[ pMsg->m_idxLane ];

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,8 @@ class CSteamNetworkConnectionBase : public ILockableThinker< ConnectionLock >
368368
/// Send a message
369369
EResult APISendMessageToConnection( const void *pData, uint32 cbData, int nSendFlags, int64 *pOutMessageNumber );
370370

371-
/// Send a message. Returns the assigned message number, or a negative EResult value
371+
/// Send a message. Returns the assigned message number, or a negative EResult value.
372+
/// Does NOT Release the message on failure -- the caller is responsible for that.
372373
int64 APISendMessageToConnection( CSteamNetworkingMessage *pMsg, SteamNetworkingMicroseconds usecNow, bool *pbThinkImmediately = nullptr );
373374

374375
/// Flush any messages queued for Nagle

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_snp.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,6 @@ int64 CSteamNetworkConnectionBase::SNP_SendMessage( CSteamNetworkingMessage *pSe
312312
if ( (size_t)pSendMessage->m_idxLane >= m_senderState.m_vecLanes.size() )
313313
{
314314
SpewBug( "Invalid lane %d. Only %d lanes configured\n", (int)pSendMessage->m_idxLane, (int)m_senderState.m_vecLanes.size() );
315-
pSendMessage->Release();
316315
return -k_EResultInvalidParam;
317316
}
318317
SSNPSenderState::Lane &lane = m_senderState.m_vecLanes[ pSendMessage->m_idxLane ];
@@ -321,7 +320,6 @@ int64 CSteamNetworkConnectionBase::SNP_SendMessage( CSteamNetworkingMessage *pSe
321320
if ( m_senderState.PendingBytesTotal() + cbData > m_connectionConfig.SendBufferSize.Get() )
322321
{
323322
SpewWarningRateLimited( usecNow, "Connection already has %u bytes pending, cannot queue any more messages\n", m_senderState.PendingBytesTotal() );
324-
pSendMessage->Release();
325323
return -k_EResultLimitExceeded;
326324
}
327325

0 commit comments

Comments
 (0)