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

include/steam/isteamnetworkingsockets.h

@@ -285,13 +285,12 @@ public:
 	/// You MUST also fill in:
 	/// - m_conn - the handle of the connection to send the message to
 	/// - m_nFlags - bitmask of k_nSteamNetworkingSend_xxx flags.
+	/// - m_idxLane - the lane to send the message on.  AllocateMessage
+	///   will set this to zero, so you can ignore this if you are not using
+	///   multiple lanes.
 	///
 	/// All other fields are currently reserved and should not be modified.
 	///
-	/// The library will take ownership of the message structures.  They may
-	/// be modified or become invalid at any time, so you must not read them
-	/// after passing them to this function.
-	///
 	/// pOutMessageNumberOrResult is an optional array that will receive,
 	/// for each message, the message number that was assigned to the message
 	/// if sending was successful.  If sending failed, then a negative EResult
@@ -299,7 +298,28 @@ public:
 	/// -k_EResultInvalidState if the connection was in an invalid state.
 	/// See ISteamNetworkingSockets::SendMessageToConnection for possible
 	/// failure codes.
-	virtual void SendMessages( int nMessages, SteamNetworkingMessage_t *const *pMessages, int64 *pOutMessageNumberOrResult ) = 0;
+	///
+	/// Once a message fails to send on a connection, any further messages
+	/// in the array going to the same connection will not be attempted.  The
+	/// pOutMessageNumberOrResult for such message will always be set to 0.
+	/// (Note that 0 is never used as a message number.)
+	///
+	/// bDeleteFailedMessages determines what happens to messages that
+	/// fail to send:
+	///
+	/// - false: Your pointer array will be modified, and the pointers
+	///     to messages that were successfully queued will be replaced with
+	///     nullptr.  The library has taken ownership and you must not access
+	///     them.  They will be released by the library when they are no longer
+	///     needed.
+	///     Any messages that were not queued (either failed to send, or were
+	///     not attempted because an earlier message for the same connection failed)
+	///     will be left in place.  You can release these messages or try to send
+	///     them later.
+	/// - true: The caller's pointer array is not modified, and the library assumes
+	///     ownership of all messages.  Messages that fail or are not attempted due
+	///     to earlier failure on the same connection will be released immediately.
+	virtual void SendMessages( int nMessages, SteamNetworkingMessage_t **pMessages, int64 *pOutMessageNumberOrResult, bool bDeleteFailedMessages ) = 0;
 
 	/// Flush any messages waiting on the Nagle timer and send them
 	/// at the next transmission opportunity (often that means right now).

include/steam/steamnetworkingsockets_flat.h

@@ -34,7 +34,7 @@ STEAMNETWORKINGSOCKETS_INTERFACE int64 SteamAPI_ISteamNetworkingSockets_GetConne
 STEAMNETWORKINGSOCKETS_INTERFACE void SteamAPI_ISteamNetworkingSockets_SetConnectionName( ISteamNetworkingSockets* self, HSteamNetConnection hPeer, const char * pszName );
 STEAMNETWORKINGSOCKETS_INTERFACE bool SteamAPI_ISteamNetworkingSockets_GetConnectionName( ISteamNetworkingSockets* self, HSteamNetConnection hPeer, char * pszName, int nMaxLen );
 STEAMNETWORKINGSOCKETS_INTERFACE EResult SteamAPI_ISteamNetworkingSockets_SendMessageToConnection( ISteamNetworkingSockets* self, HSteamNetConnection hConn, const void * pData, uint32 cbData, int nSendFlags, int64 * pOutMessageNumber );
-STEAMNETWORKINGSOCKETS_INTERFACE void SteamAPI_ISteamNetworkingSockets_SendMessages( ISteamNetworkingSockets* self, int nMessages, SteamNetworkingMessage_t *const * pMessages, int64 * pOutMessageNumberOrResult );
+STEAMNETWORKINGSOCKETS_INTERFACE void SteamAPI_ISteamNetworkingSockets_SendMessages( ISteamNetworkingSockets* self, int nMessages, SteamNetworkingMessage_t ** pMessages, int64 * pOutMessageNumberOrResult, bool bDeleteFailedMessages );
 STEAMNETWORKINGSOCKETS_INTERFACE EResult SteamAPI_ISteamNetworkingSockets_FlushMessagesOnConnection( ISteamNetworkingSockets* self, HSteamNetConnection hConn );
 STEAMNETWORKINGSOCKETS_INTERFACE int SteamAPI_ISteamNetworkingSockets_ReceiveMessagesOnConnection( ISteamNetworkingSockets* self, HSteamNetConnection hConn, SteamNetworkingMessage_t ** ppOutMessages, int nMaxMessages );
 STEAMNETWORKINGSOCKETS_INTERFACE bool SteamAPI_ISteamNetworkingSockets_GetConnectionInfo( ISteamNetworkingSockets* self, HSteamNetConnection hConn, SteamNetConnectionInfo_t * pInfo );

src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp

@@ -1271,7 +1271,7 @@ EResult CSteamNetworkingSockets::SendMessageToConnection( HSteamNetConnection hC
 	return pConn->APISendMessageToConnection( pData, cbData, nSendFlags, pOutMessageNumber );
 }
 
-void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessage_t *const *pMessages, int64 *pOutMessageNumberOrResult )
+void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessage_t **pMessages, int64 *pOutMessageNumberOrResult, bool bDeleteFailedMessages )
 {
 
 	// Get list of messages, grouped by connection.
@@ -1307,7 +1307,8 @@ void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessag
 		{
 			if ( pOutMessageNumberOrResult )
 				pOutMessageNumberOrResult[i] = -k_EResultInvalidParam;
-			pMsg->Release();
+			if ( bDeleteFailedMessages )
+				pMsg->Release();
 			continue;
 		}
 
@@ -1331,6 +1332,7 @@ void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessag
 	HSteamNetConnection hConn = k_HSteamNetConnection_Invalid;
 	ConnectionScopeLock connectionLock;
 	bool bConnectionThinkImmediately = false;
+	bool bCurrentConnectionFailed = false;
 	for ( SortMsg_t *pSort = pSortMessages ; pSort < pSortEnd ; ++pSort )
 	{
 
@@ -1350,13 +1352,20 @@ void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessag
 			// Locate the connection
 			hConn = pSort->m_hConn;
 			pConn = GetConnectionByHandleForAPI( hConn, connectionLock, "SendMessages" );
+			bCurrentConnectionFailed = false;
 		}
 
-		CSteamNetworkingMessage *pMsg = static_cast<CSteamNetworkingMessage*>( pMessages[pSort->m_idx] );
+		const int idx = pSort->m_idx;
+		CSteamNetworkingMessage *pMsg = static_cast<CSteamNetworkingMessage*>( pMessages[idx] );
 
-		// Current connection is valid?
+		// Once a message fails on a connection, subsequent messages to that connection
+		// are not attempted.  Result stays 0 (set by the memset above).
 		int64 result;
-		if ( pConn )
+		if ( bCurrentConnectionFailed )
+		{
+			result = 0;
+		}
+		else if ( pConn )
 		{
 
 			// Attempt to send
@@ -1364,18 +1373,30 @@ void CSteamNetworkingSockets::SendMessages( int nMessages, SteamNetworkingMessag
 			result = pConn->APISendMessageToConnection( pMsg, usecNow, &bThinkImmediately );
 			if ( bThinkImmediately )
 				bConnectionThinkImmediately = true;
-			if ( result <= 0 )
+			if ( result > 0 )
-				pMsg->Release();
+			{
+				// Successfully queued.  Clear pointer to let caller know, if
+				// they requested this mode of operation.
+				if ( !bDeleteFailedMessages )
+					pMessages[idx] = nullptr;
+			}
 		}
 		else
 		{
-			pMsg->Release();
+			// Connection handle not found -- first message reports the error;
+			// subsequent messages to the same connection get result=0 (not attempted).
 			result = -k_EResultInvalidParam;
 		}
 
-		// Return result for this message if they asked for it
+		if ( result <= 0 )
+		{
+			bCurrentConnectionFailed = true;
+			if ( bDeleteFailedMessages )
+				pMsg->Release();
+		}
+
 		if ( pOutMessageNumberOrResult )
-			pOutMessageNumberOrResult[pSort->m_idx] = result;
+			pOutMessageNumberOrResult[idx] = result;
 	}
 
 	// Flush out last connection, if any

src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.h

@@ -92,7 +92,7 @@ public:
 	virtual void SetConnectionName( HSteamNetConnection hPeer, const char *pszName ) override;
 	virtual bool GetConnectionName( HSteamNetConnection hPeer, char *pszName, int nMaxLen ) override;
 	virtual EResult SendMessageToConnection( HSteamNetConnection hConn, const void *pData, uint32 cbData, int nSendFlags, int64 *pOutMessageNumber ) override;
-	virtual void SendMessages( int nMessages, SteamNetworkingMessage_t *const *pMessages, int64 *pOutMessageNumberOrResult ) override;
+	virtual void SendMessages( int nMessages, SteamNetworkingMessage_t **pMessages, int64 *pOutMessageNumberOrResult, bool bDeleteFailedMessages ) override;
 	virtual EResult FlushMessagesOnConnection( HSteamNetConnection hConn ) override;
 	virtual int ReceiveMessagesOnConnection( HSteamNetConnection hConn, SteamNetworkingMessage_t **ppOutMessages, int nMaxMessages ) override;
 	virtual bool GetConnectionInfo( HSteamNetConnection hConn, SteamNetConnectionInfo_t *pInfo ) override;

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_flat.cpp

@@ -65,9 +65,9 @@ STEAMNETWORKINGSOCKETS_INTERFACE EResult SteamAPI_ISteamNetworkingSockets_SendMe
 {
 	return self->SendMessageToConnection( hConn,pData,cbData,nSendFlags,pOutMessageNumber );
 }
-STEAMNETWORKINGSOCKETS_INTERFACE void SteamAPI_ISteamNetworkingSockets_SendMessages( ISteamNetworkingSockets* self, int nMessages, SteamNetworkingMessage_t *const * pMessages, int64 * pOutMessageNumberOrResult )
+STEAMNETWORKINGSOCKETS_INTERFACE void SteamAPI_ISteamNetworkingSockets_SendMessages( ISteamNetworkingSockets* self, int nMessages, SteamNetworkingMessage_t ** pMessages, int64 * pOutMessageNumberOrResult, bool bDeleteFailedMessages )
 {
-	self->SendMessages( nMessages,pMessages,pOutMessageNumberOrResult );
+	self->SendMessages( nMessages,pMessages,pOutMessageNumberOrResult,bDeleteFailedMessages );
 }
 STEAMNETWORKINGSOCKETS_INTERFACE EResult SteamAPI_ISteamNetworkingSockets_FlushMessagesOnConnection( ISteamNetworkingSockets* self, HSteamNetConnection hConn )
 {

tests/test_connection.cpp

@@ -717,7 +717,7 @@ void Test_lane_quick_queueanddrain()
 			}
 		}
 		assert( idxMsg == k_nTotalMsg );
-		SteamNetworkingSockets()->SendMessages( idxMsg, pMessages, nullptr );
+		SteamNetworkingSockets()->SendMessages( idxMsg, pMessages, nullptr, true );
 	}
 
 	// Remember when we sent all the messages
@@ -785,7 +785,7 @@ void Test_lane_quick_queueanddrain()
 			pMsg->m_idxLane = (uint16)idxLane;
 			pMessages[idxLane] = pMsg;
 		}
-		SteamNetworkingSockets()->SendMessages( k_nLanes, pMessages, nullptr );
+		SteamNetworkingSockets()->SendMessages( k_nLanes, pMessages, nullptr, true );
 	}
 
 	int cbLaneReceived[k_nLanes] = {};
@@ -959,7 +959,7 @@ void Test_lane_quick_priority_and_background()
 			*(SteamNetworkingMicroseconds *)pMsg->m_pData = usecNow;
 
 			int64 nMsgNum;
-			SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum );
+			SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum, true );
 			++nMsgSent[k_LaneBackground];
 			assert( nMsgNum == nMsgSent[k_LaneBackground] );
 		}
@@ -977,7 +977,7 @@ void Test_lane_quick_priority_and_background()
 			*(SteamNetworkingMicroseconds *)pMsg->m_pData = usecNow;
 
 			int64 nMsgNum;
-			SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum );
+			SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum, true );
 			++nMsgSent[k_LaneUrgent];
 			assert( nMsgNum == nMsgSent[k_LaneUrgent] );
 
@@ -1003,7 +1003,7 @@ void Test_lane_quick_priority_and_background()
 				*(SteamNetworkingMicroseconds *)pMsg->m_pData = usecNow;
 
 				int64 nMsgNum;
-				SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum );
+				SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum, true );
 				++nMsgSent[k_LaneGameplay];
 				assert( nMsgNum == nMsgSent[k_LaneGameplay] );
 			}
@@ -1018,7 +1018,7 @@ void Test_lane_quick_priority_and_background()
 				pMsg->m_nFlags = std::uniform_int_distribution<>( 0, 100 )( g_rand ) < 30 ? k_nSteamNetworkingSend_ReliableNoNagle : k_nSteamNetworkingSend_UnreliableNoNagle;
 
 				int64 nMsgNum;
-				SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum );
+				SteamNetworkingSockets()->SendMessages( 1, &pMsg, &nMsgNum, true );
 				assert( nMsgNum >= 0 );
 			}
 
@@ -1134,7 +1134,7 @@ void Test_pipe()
 		void *pSendData = pSendMsg->m_pData;
 
 		int64 nMsgNum;
-		SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNum );
+		SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNum, true );
 		assert( nMsgNum > 0 );
 
 		SteamNetworkingMessage_t *pRecvMsg = nullptr;
@@ -1153,7 +1153,7 @@ void Test_pipe()
 		pSendMsg->m_nFlags = k_nSteamNetworkingSend_Reliable;
 
 		int64 nMsgNum;
-		SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNum );
+		SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNum, true );
 		assert( nMsgNum > 0 );
 
 		SteamNetworkingMessage_t *pRecvMsg = nullptr;
@@ -1265,7 +1265,7 @@ void Test_netloopback_throughput()
 					// Don't bother initializing the body
 
 					int64 nMsgNumberOrResult;
-					SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNumberOrResult );
+					SteamNetworkingSockets()->SendMessages( 1, &pSendMsg, &nMsgNumberOrResult, true );
 					if ( nMsgNumberOrResult == -k_EResultLimitExceeded )
 					{
 						TEST_Printf( "SendMessage returned limit exceeded trying to queue %d + %d = %d\n", serverStatus.m_cbPendingReliable, cbSendMsg, serverStatus.m_cbPendingReliable + cbSendMsg );