Skip to content

Commit 4fbfe83

Browse files
zpostfactozpostfacto
committed
Semantic change to ISteamNetworkingSockets::SendMessages
Added bDeleteFailedMessages. By setting this to false, the caller has the opportunity to retry failed messages, which is an especially reasonably thing to do in the case of the send buffer being full. Also clarified expected (and enforced) expected behaviour on how subsequent messages on a connection are handled after a failed message send. Previously, there was a fairly significant bug, which is that we would coutinue to try to send messages, which is bad because it can break fundamental guarantees about message delivery order. Now, we will always stop at the first failure. This fixes issue #317
1 parent 3b03992 commit 4fbfe83

6 files changed

Lines changed: 69 additions & 28 deletions

File tree

‎include/steam/isteamnetworkingsockets.h‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -285,21 +285,41 @@ class ISteamNetworkingSockets
285285
/// You MUST also fill in:
286286
/// - m_conn - the handle of the connection to send the message to
287287
/// - m_nFlags - bitmask of k_nSteamNetworkingSend_xxx flags.
288+
/// - m_idxLane - the lane to send the message on. AllocateMessage
289+
/// will set this to zero, so you can ignore this if you are not using
290+
/// multiple lanes.
288291
///
289292
/// All other fields are currently reserved and should not be modified.
290293
///
291-
/// The library will take ownership of the message structures. They may
292-
/// be modified or become invalid at any time, so you must not read them
293-
/// after passing them to this function.
294-
///
295294
/// pOutMessageNumberOrResult is an optional array that will receive,
296295
/// for each message, the message number that was assigned to the message
297296
/// if sending was successful. If sending failed, then a negative EResult
298297
/// value is placed into the array. For example, the array will hold
299298
/// -k_EResultInvalidState if the connection was in an invalid state.
300299
/// See ISteamNetworkingSockets::SendMessageToConnection for possible
301300
/// failure codes.
302-
virtual void SendMessages( int nMessages, SteamNetworkingMessage_t *const *pMessages, int64 *pOutMessageNumberOrResult ) = 0;
301+
///
302+
/// Once a message fails to send on a connection, any further messages
303+
/// in the array going to the same connection will not be attempted. The
304+
/// pOutMessageNumberOrResult for such message will always be set to 0.
305+
/// (Note that 0 is never used as a message number.)
306+
///
307+
/// bDeleteFailedMessages determines what happens to messages that
308+
/// fail to send:
309+
///
310+
/// - false: Your pointer array will be modified, and the pointers
311+
/// to messages that were successfully queued will be replaced with
312+
/// nullptr. The library has taken ownership and you must not access
313+
/// them. They will be released by the library when they are no longer
314+
/// needed.
315+
/// Any messages that were not queued (either failed to send, or were
316+
/// not attempted because an earlier message for the same connection failed)
317+
/// will be left in place. You can release these messages or try to send
318+
/// them later.
319+
/// - true: The caller's pointer array is not modified, and the library assumes
320+
/// ownership of all messages. Messages that fail or are not attempted due
321+
/// to earlier failure on the same connection will be released immediately.
322+
virtual void SendMessages( int nMessages, SteamNetworkingMessage_t **pMessages, int64 *pOutMessageNumberOrResult, bool bDeleteFailedMessages ) = 0;
303323

304324
/// Flush any messages waiting on the Nagle timer and send them
305325
/// at the next transmission opportunity (often that means right now).

‎include/steam/steamnetworkingsockets_flat.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ STEAMNETWORKINGSOCKETS_INTERFACE int64 SteamAPI_ISteamNetworkingSockets_GetConne
3434
STEAMNETWORKINGSOCKETS_INTERFACE void SteamAPI_ISteamNetworkingSockets_SetConnectionName( ISteamNetworkingSockets* self, HSteamNetConnection hPeer, const char * pszName );
3535
STEAMNETWORKINGSOCKETS_INTERFACE bool SteamAPI_ISteamNetworkingSockets_GetConnectionName( ISteamNetworkingSockets* self, HSteamNetConnection hPeer, char * pszName, int nMaxLen );
3636
STEAMNETWORKINGSOCKETS_INTERFACE EResult SteamAPI_ISteamNetworkingSockets_SendMessageToConnection( ISteamNetworkingSockets* self, HSteamNetConnection hConn, const void * pData, uint32 cbData, int nSendFlags, int64 * pOutMessageNumber );
37-
STEAMNETWORKINGSOCKETS_INTERFACE void SteamAPI_ISteamNetworkingSockets_SendMessages( ISteamNetworkingSockets* self, int nMessages, SteamNetworkingMessage_t *const * pMessages, int64 * pOutMessageNumberOrResult );
37+
STEAMNETWORKINGSOCKETS_INTERFACE void SteamAPI_ISteamNetworkingSockets_SendMessages( ISteamNetworkingSockets* self, int nMessages, SteamNetworkingMessage_t ** pMessages, int64 * pOutMessageNumberOrResult, bool bDeleteFailedMessages );
3838
STEAMNETWORKINGSOCKETS_INTERFACE EResult SteamAPI_ISteamNetworkingSockets_FlushMessagesOnConnection( ISteamNetworkingSockets* self, HSteamNetConnection hConn );
3939
STEAMNETWORKINGSOCKETS_INTERFACE int SteamAPI_ISteamNetworkingSockets_ReceiveMessagesOnConnection( ISteamNetworkingSockets* self, HSteamNetConnection hConn, SteamNetworkingMessage_t ** ppOutMessages, int nMaxMessages );
4040
STEAMNETWORKINGSOCKETS_INTERFACE bool SteamAPI_ISteamNetworkingSockets_GetConnectionInfo( ISteamNetworkingSockets* self, HSteamNetConnection hConn, SteamNetConnectionInfo_t * pInfo );

‎src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,7 +1271,7 @@ EResult CSteamNetworkingSockets::SendMessageToConnection( HSteamNetConnection hC
12711271
return pConn->APISendMessageToConnection( pData, cbData, nSendFlags, pOutMessageNumber );
12721272
}
12731273

1274-
void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessage_t *const *pMessages, int64 *pOutMessageNumberOrResult )
1274+
void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessage_t **pMessages, int64 *pOutMessageNumberOrResult, bool bDeleteFailedMessages )
12751275
{
12761276

12771277
// Get list of messages, grouped by connection.
@@ -1307,7 +1307,8 @@ void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessag
13071307
{
13081308
if ( pOutMessageNumberOrResult )
13091309
pOutMessageNumberOrResult[i] = -k_EResultInvalidParam;
1310-
pMsg->Release();
1310+
if ( bDeleteFailedMessages )
1311+
pMsg->Release();
13111312
continue;
13121313
}
13131314

@@ -1331,6 +1332,7 @@ void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessag
13311332
HSteamNetConnection hConn = k_HSteamNetConnection_Invalid;
13321333
ConnectionScopeLock connectionLock;
13331334
bool bConnectionThinkImmediately = false;
1335+
bool bCurrentConnectionFailed = false;
13341336
for ( SortMsg_t *pSort = pSortMessages ; pSort < pSortEnd ; ++pSort )
13351337
{
13361338

@@ -1350,32 +1352,51 @@ void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessag
13501352
// Locate the connection
13511353
hConn = pSort->m_hConn;
13521354
pConn = GetConnectionByHandleForAPI( hConn, connectionLock, "SendMessages" );
1355+
bCurrentConnectionFailed = false;
13531356
}
13541357

1355-
CSteamNetworkingMessage *pMsg = static_cast<CSteamNetworkingMessage*>( pMessages[pSort->m_idx] );
1358+
const int idx = pSort->m_idx;
1359+
CSteamNetworkingMessage *pMsg = static_cast<CSteamNetworkingMessage*>( pMessages[idx] );
13561360

1357-
// Current connection is valid?
1361+
// Once a message fails on a connection, subsequent messages to that connection
1362+
// are not attempted. Result stays 0 (set by the memset above).
13581363
int64 result;
1359-
if ( pConn )
1364+
if ( bCurrentConnectionFailed )
1365+
{
1366+
result = 0;
1367+
}
1368+
else if ( pConn )
13601369
{
13611370

13621371
// Attempt to send
13631372
bool bThinkImmediately = false;
13641373
result = pConn->APISendMessageToConnection( pMsg, usecNow, &bThinkImmediately );
13651374
if ( bThinkImmediately )
13661375
bConnectionThinkImmediately = true;
1367-
if ( result <= 0 )
1368-
pMsg->Release();
1376+
if ( result > 0 )
1377+
{
1378+
// Successfully queued. Clear pointer to let caller know, if
1379+
// they requested this mode of operation.
1380+
if ( !bDeleteFailedMessages )
1381+
pMessages[idx] = nullptr;
1382+
}
13691383
}
13701384
else
13711385
{
1372-
pMsg->Release();
1386+
// Connection handle not found -- first message reports the error;
1387+
// subsequent messages to the same connection get result=0 (not attempted).
13731388
result = -k_EResultInvalidParam;
13741389
}
13751390

1376-
// Return result for this message if they asked for it
1391+
if ( result <= 0 )
1392+
{
1393+
bCurrentConnectionFailed = true;
1394+
if ( bDeleteFailedMessages )
1395+
pMsg->Release();
1396+
}
1397+
13771398
if ( pOutMessageNumberOrResult )
1378-
pOutMessageNumberOrResult[pSort->m_idx] = result;
1399+
pOutMessageNumberOrResult[idx] = result;
13791400
}
13801401

13811402
// Flush out last connection, if any

‎src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ class CSteamNetworkingSockets : public IClientNetworkingSockets
9292
virtual void SetConnectionName( HSteamNetConnection hPeer, const char *pszName ) override;
9393
virtual bool GetConnectionName( HSteamNetConnection hPeer, char *pszName, int nMaxLen ) override;
9494
virtual EResult SendMessageToConnection( HSteamNetConnection hConn, const void *pData, uint32 cbData, int nSendFlags, int64 *pOutMessageNumber ) override;
95-
virtual void SendMessages( int nMessages, SteamNetworkingMessage_t *const *pMessages, int64 *pOutMessageNumberOrResult ) override;
95+
virtual void SendMessages( int nMessages, SteamNetworkingMessage_t **pMessages, int64 *pOutMessageNumberOrResult, bool bDeleteFailedMessages ) override;
9696
virtual EResult FlushMessagesOnConnection( HSteamNetConnection hConn ) override;
9797
virtual int ReceiveMessagesOnConnection( HSteamNetConnection hConn, SteamNetworkingMessage_t **ppOutMessages, int nMaxMessages ) override;
9898
virtual bool GetConnectionInfo( HSteamNetConnection hConn, SteamNetConnectionInfo_t *pInfo ) override;

‎src/steamnetworkingsockets/clientlib/steamnetworkingsockets_flat.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ STEAMNETWORKINGSOCKETS_INTERFACE EResult SteamAPI_ISteamNetworkingSockets_SendMe
6565
{
6666
return self->SendMessageToConnection( hConn,pData,cbData,nSendFlags,pOutMessageNumber );
6767
}
68-
STEAMNETWORKINGSOCKETS_INTERFACE void SteamAPI_ISteamNetworkingSockets_SendMessages( ISteamNetworkingSockets* self, int nMessages, SteamNetworkingMessage_t *const * pMessages, int64 * pOutMessageNumberOrResult )
68+
STEAMNETWORKINGSOCKETS_INTERFACE void SteamAPI_ISteamNetworkingSockets_SendMessages( ISteamNetworkingSockets* self, int nMessages, SteamNetworkingMessage_t ** pMessages, int64 * pOutMessageNumberOrResult, bool bDeleteFailedMessages )
6969
{
70-
self->SendMessages( nMessages,pMessages,pOutMessageNumberOrResult );
70+
self->SendMessages( nMessages,pMessages,pOutMessageNumberOrResult,bDeleteFailedMessages );
7171
}
7272
STEAMNETWORKINGSOCKETS_INTERFACE EResult SteamAPI_ISteamNetworkingSockets_FlushMessagesOnConnection( ISteamNetworkingSockets* self, HSteamNetConnection hConn )
7373
{

‎tests/test_connection.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ void Test_lane_quick_queueanddrain()
717717
}
718718
}
719719
assert( idxMsg == k_nTotalMsg );
720-
SteamNetworkingSockets()->SendMessages( idxMsg, pMessages, nullptr );
720+
SteamNetworkingSockets()->SendMessages( idxMsg, pMessages, nullptr, true );
721721
}
722722

723723
// Remember when we sent all the messages
@@ -785,7 +785,7 @@ void Test_lane_quick_queueanddrain()
785785
pMsg->m_idxLane = (uint16)idxLane;
786786
pMessages[idxLane] = pMsg;
787787
}
788-
SteamNetworkingSockets()->SendMessages( k_nLanes, pMessages, nullptr );
788+
SteamNetworkingSockets()->SendMessages( k_nLanes, pMessages, nullptr, true );
789789
}
790790

791791
int cbLaneReceived[k_nLanes] = {};
@@ -959,7 +959,7 @@ void Test_lane_quick_priority_and_background()
959959
*(SteamNetworkingMicroseconds *)pMsg->m_pData = usecNow;
960960

961961
int64 nMsgNum;
962-
SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum );
962+
SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum, true );
963963
++nMsgSent[k_LaneBackground];
964964
assert( nMsgNum == nMsgSent[k_LaneBackground] );
965965
}
@@ -977,7 +977,7 @@ void Test_lane_quick_priority_and_background()
977977
*(SteamNetworkingMicroseconds *)pMsg->m_pData = usecNow;
978978

979979
int64 nMsgNum;
980-
SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum );
980+
SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum, true );
981981
++nMsgSent[k_LaneUrgent];
982982
assert( nMsgNum == nMsgSent[k_LaneUrgent] );
983983

@@ -1003,7 +1003,7 @@ void Test_lane_quick_priority_and_background()
10031003
*(SteamNetworkingMicroseconds *)pMsg->m_pData = usecNow;
10041004

10051005
int64 nMsgNum;
1006-
SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum );
1006+
SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum, true );
10071007
++nMsgSent[k_LaneGameplay];
10081008
assert( nMsgNum == nMsgSent[k_LaneGameplay] );
10091009
}
@@ -1018,7 +1018,7 @@ void Test_lane_quick_priority_and_background()
10181018
pMsg->m_nFlags = std::uniform_int_distribution<>( 0, 100 )( g_rand ) < 30 ? k_nSteamNetworkingSend_ReliableNoNagle : k_nSteamNetworkingSend_UnreliableNoNagle;
10191019

10201020
int64 nMsgNum;
1021-
SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum );
1021+
SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum, true );
10221022
assert( nMsgNum >= 0 );
10231023
}
10241024

@@ -1134,7 +1134,7 @@ void Test_pipe()
11341134
void *pSendData = pSendMsg->m_pData;
11351135

11361136
int64 nMsgNum;
1137-
SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNum );
1137+
SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNum, true );
11381138
assert( nMsgNum > 0 );
11391139

11401140
SteamNetworkingMessage_t *pRecvMsg = nullptr;
@@ -1153,7 +1153,7 @@ void Test_pipe()
11531153
pSendMsg->m_nFlags = k_nSteamNetworkingSend_Reliable;
11541154

11551155
int64 nMsgNum;
1156-
SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNum );
1156+
SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNum, true );
11571157
assert( nMsgNum > 0 );
11581158

11591159
SteamNetworkingMessage_t *pRecvMsg = nullptr;
@@ -1265,7 +1265,7 @@ void Test_netloopback_throughput()
12651265
// Don't bother initializing the body
12661266

12671267
int64 nMsgNumberOrResult;
1268-
SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNumberOrResult );
1268+
SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNumberOrResult, true );
12691269
if ( nMsgNumberOrResult == -k_EResultLimitExceeded )
12701270
{
12711271
TEST_Printf( "SendMessage returned limit exceeded trying to queue %d + %d = %d\n", serverStatus.m_cbPendingReliable, cbSendMsg, serverStatus.m_cbPendingReliable + cbSendMsg );

0 commit comments

Comments
 (0)