Add k_ESteamNetworkingConfig_ConnectionUserData

Now we store the userdata in a config value, so that it can be set
atomically when a connection is created, and also so that the default
value of -1 can be customized.

Also added some warnings about the dangers of using the userData
field in callback structs.  I worry I've created a footgun here, and
would be tempted to remove the field entirely from
SteamNetConnectionStatusChangedCallback_t, but it is coming in
through a member struct SteamNetConnectionInfo_t, and it makes
sense there.

Addresses problems discussed in issue #162.

P4:6354936

include/steam/isteamnetworkingsockets.h

@@ -192,7 +192,23 @@ public:
 	/// Set connection user data.  the data is returned in the following places
 	/// - You can query it using GetConnectionUserData.
 	/// - The SteamNetworkingmessage_t structure.
-	/// - The SteamNetConnectionInfo_t structure.  (Which is a member of SteamNetConnectionStatusChangedCallback_t.)
+	/// - The SteamNetConnectionInfo_t structure.
+	///   (Which is a member of SteamNetConnectionStatusChangedCallback_t -- but see WARNINGS below!!!!)
+	///
+	/// Do you need to set this atomically when the connection is created?
+	/// See k_ESteamNetworkingConfig_ConnectionUserData.
+	///
+	/// WARNING: Be *very careful* when using the value provided in callbacks structs.
+	/// Callbacks are queued, and the value that you will receive in your
+	/// callback is the userdata that was effective at the time the callback
+	/// was queued.  There are subtle race conditions that can hapen if you
+	/// don't understand this!
+	///
+	/// If any incoming messages for this connection are queued, the userdata
+	/// field is updated, so that when when you receive messages (e.g. with
+	/// ReceiveMessagesOnConnection), they will always have the very latest
+	/// userdata.  So the tricky race conditions that can happen with callbacks
+	/// do not apply to retrieving messages.
 	///
 	/// Returns false if the handle is invalid.
 	virtual bool SetConnectionUserData( HSteamNetConnection hPeer, int64 nUserData ) = 0;

include/steam/steamnetworkingtypes.h

@@ -1129,6 +1129,41 @@ enum ESteamNetworkingConfigValue
 	/// Default is 512k (524288 bytes)
 	k_ESteamNetworkingConfig_SendBufferSize = 9,
 
+	/// [connection int64] Get/set userdata as a configuration option.
+	/// The default value is -1.   You may want to set the user data as
+	/// a config value, instead of using ISteamNetworkingSockets::SetConnectionUserData
+	/// in two specific instances:
+	///
+	/// - You wish to set the userdata atomically when creating
+	///   an outbound connection, so that the userdata is filled in properly
+	///   for any callbacks that happen.  However, note that this trick
+	///   only works for connections initiated locally!  For incoming
+	///   connections, multiple state transitions may happen and
+	///   callbacks be queued, before you are able to service the first
+	///   callback!  Be careful!
+	///
+	/// - You can set the default userdata for all newly created connections
+	///   by setting this value at a higher level (e.g. on the listen
+	///   socket or at the global level.)  Then this default
+	///   value will be inherited when the connection is created.
+	///   This is useful in case -1 is a valid userdata value, and you
+	///   wish to use something else as the default value so you can
+	///   tell if it has been set or not.
+	///
+	///   HOWEVER: once a connection is created, the effective value is
+	///   then bound to the connection.  Unlike other connection options,
+	///   if you change it again at a higher level, the new value will not
+	///   be inherited by connections.
+	///
+	/// Using the userdata field in callback structs is not advised because
+	/// of tricky race conditions.  Instead, you might try one of these methods:
+	///
+	/// - Use a separate map with the HSteamNetConnection as the key.
+	/// - Fetch the userdata from the connection in your callback
+	///   using ISteamNetworkingSockets::GetConnectionUserData, to
+	//    ensure you have the current value.
+	k_ESteamNetworkingConfig_ConnectionUserData = 40,
+
 	/// [connection int32] Minimum/maximum send rate clamp, 0 is no limit.
 	/// This value will control the min/max allowed sending rate that 
 	/// bandwidth estimation is allowed to reach.  Default is 0 (no-limit)

src/steamnetworkingsockets/clientlib/csteamnetworkingsockets.cpp

@@ -58,6 +58,7 @@ DEFINE_GLOBAL_CONFIGVAL( void *, Callback_CreateConnectionSignaling, nullptr );
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, TimeoutInitial, 10000, 0, INT32_MAX );
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, TimeoutConnected, 10000, 0, INT32_MAX );
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendBufferSize, 512*1024, 0, 0x10000000 );
+DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int64, ConnectionUserData, -1 ); // no limits here
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendRateMin, 128*1024, 1024, 0x10000000 );
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, SendRateMax, 1024*1024, 1024, 0x10000000 );
 DEFINE_CONNECTON_DEFAULT_CONFIGVAL( int32, NagleTime, 5000, 0, 20000 );
@@ -1459,12 +1460,12 @@ static ConfigValue<T> *EvaluateScopeConfigValue( GlobalConfigValueEntry *pEntry,
 	return nullptr;
 }
 
-static bool AssignConfigValueTyped( ConfigValue<int32> *pVal, ESteamNetworkingConfigDataType eDataType, const void *pArg )
+static bool AssignConfigValueTyped( int32 *pVal, ESteamNetworkingConfigDataType eDataType, const void *pArg )
 {
 	switch ( eDataType )
 	{
 		case k_ESteamNetworkingConfig_Int32:
-			pVal->m_data = *(int32*)pArg;
+			*pVal = *(int32*)pArg;
 			break;
 
 		case k_ESteamNetworkingConfig_Int64:
@@ -1472,12 +1473,12 @@ static bool AssignConfigValueTyped( ConfigValue<int32> *pVal, ESteamNetworkingCo
 			int64 arg = *(int64*)pArg;
 			if ( (int32)arg != arg )
 				return false; // Cannot truncate!
-			pVal->m_data = *(int32*)arg;
+			*pVal = *(int32*)arg;
 			break;
 		}
 
 		case k_ESteamNetworkingConfig_Float:
-			pVal->m_data = (int32)floor( *(float*)pArg + .5f );
+			*pVal = (int32)floor( *(float*)pArg + .5f );
 			break;
 
 		case k_ESteamNetworkingConfig_String:
@@ -1485,7 +1486,7 @@ static bool AssignConfigValueTyped( ConfigValue<int32> *pVal, ESteamNetworkingCo
 			int x;
 			if ( sscanf( (const char *)pArg, "%d", &x ) != 1 )
 				return false;
-			pVal->m_data = x;
+			*pVal = x;
 			break;
 		}
 
@@ -1496,22 +1497,22 @@ static bool AssignConfigValueTyped( ConfigValue<int32> *pVal, ESteamNetworkingCo
 	return true;
 }
 
-static bool AssignConfigValueTyped( ConfigValue<int64> *pVal, ESteamNetworkingConfigDataType eDataType, const void *pArg )
+static bool AssignConfigValueTyped( int64 *pVal, ESteamNetworkingConfigDataType eDataType, const void *pArg )
 {
 	switch ( eDataType )
 	{
 		case k_ESteamNetworkingConfig_Int32:
-			pVal->m_data = *(int32*)pArg;
+			*pVal = *(int32*)pArg;
 			break;
 
 		case k_ESteamNetworkingConfig_Int64:
 		{
-			pVal->m_data = *(int64*)pArg;
+			*pVal = *(int64*)pArg;
 			break;
 		}
 
 		case k_ESteamNetworkingConfig_Float:
-			pVal->m_data = (int64)floor( *(float*)pArg + .5f );
+			*pVal = (int64)floor( *(float*)pArg + .5f );
 			break;
 
 		case k_ESteamNetworkingConfig_String:
@@ -1519,7 +1520,7 @@ static bool AssignConfigValueTyped( ConfigValue<int64> *pVal, ESteamNetworkingCo
 			long long x;
 			if ( sscanf( (const char *)pArg, "%lld", &x ) != 1 )
 				return false;
-			pVal->m_data = (int64)x;
+			*pVal = (int64)x;
 			break;
 		}
 
@@ -1530,22 +1531,22 @@ static bool AssignConfigValueTyped( ConfigValue<int64> *pVal, ESteamNetworkingCo
 	return true;
 }
 
-static bool AssignConfigValueTyped( ConfigValue<float> *pVal, ESteamNetworkingConfigDataType eDataType, const void *pArg )
+static bool AssignConfigValueTyped( float *pVal, ESteamNetworkingConfigDataType eDataType, const void *pArg )
 {
 	switch ( eDataType )
 	{
 		case k_ESteamNetworkingConfig_Int32:
-			pVal->m_data = (float)( *(int32*)pArg );
+			*pVal = (float)( *(int32*)pArg );
 			break;
 
 		case k_ESteamNetworkingConfig_Int64:
 		{
-			pVal->m_data = (float)( *(int64*)pArg );
+			*pVal = (float)( *(int64*)pArg );
 			break;
 		}
 
 		case k_ESteamNetworkingConfig_Float:
-			pVal->m_data = *(float*)pArg;
+			*pVal = *(float*)pArg;
 			break;
 
 		case k_ESteamNetworkingConfig_String:
@@ -1553,7 +1554,7 @@ static bool AssignConfigValueTyped( ConfigValue<float> *pVal, ESteamNetworkingCo
 			float x;
 			if ( sscanf( (const char *)pArg, "%f", &x ) != 1 )
 				return false;
-			pVal->m_data = x;
+			*pVal = x;
 			break;
 		}
 
@@ -1564,7 +1565,7 @@ static bool AssignConfigValueTyped( ConfigValue<float> *pVal, ESteamNetworkingCo
 	return true;
 }
 
-static bool AssignConfigValueTyped( ConfigValue<std::string> *pVal, ESteamNetworkingConfigDataType eDataType, const void *pArg )
+static bool AssignConfigValueTyped( std::string *pVal, ESteamNetworkingConfigDataType eDataType, const void *pArg )
 {
 	char temp[64];
 
@@ -1572,21 +1573,21 @@ static bool AssignConfigValueTyped( ConfigValue<std::string> *pVal, ESteamNetwor
 	{
 		case k_ESteamNetworkingConfig_Int32:
 			V_sprintf_safe( temp, "%d", *(int32*)pArg );
-			pVal->m_data = temp;
+			*pVal = temp;
 			break;
 
 		case k_ESteamNetworkingConfig_Int64:
 			V_sprintf_safe( temp, "%lld", (long long)*(int64*)pArg );
-			pVal->m_data = temp;
+			*pVal = temp;
 			break;
 
 		case k_ESteamNetworkingConfig_Float:
 			V_sprintf_safe( temp, "%g", *(float*)pArg );
-			pVal->m_data = temp;
+			*pVal = temp;
 			break;
 
 		case k_ESteamNetworkingConfig_String:
-			pVal->m_data = (const char *)pArg;
+			*pVal = (const char *)pArg;
 			break;
 
 		default:
@@ -1596,12 +1597,12 @@ static bool AssignConfigValueTyped( ConfigValue<std::string> *pVal, ESteamNetwor
 	return true;
 }
 
-static bool AssignConfigValueTyped( ConfigValue<void *> *pVal, ESteamNetworkingConfigDataType eDataType, const void *pArg )
+static bool AssignConfigValueTyped( void **pVal, ESteamNetworkingConfigDataType eDataType, const void *pArg )
 {
 	switch ( eDataType )
 	{
 		case k_ESteamNetworkingConfig_Ptr:
-			pVal->m_data = *(void **)pArg;
+			*pVal = *(void **)pArg;
 			break;
 
 		default:
@@ -1637,6 +1638,12 @@ bool SetConfigValueTyped(
 			Assert( pGlobal->IsSet() );
 			pGlobal->m_data = pGlobal->m_defaultValue;
 		}
+		else if ( eScopeType == k_ESteamNetworkingConfig_Connection && pEntry->m_eValue == k_ESteamNetworkingConfig_ConnectionUserData )
+		{
+			// Once this is set, we cannot clear it or inherit it.
+			SpewError( "Cannot clear connection user data\n" );
+			return false;
+		}
 		else
 		{
 			Assert( pVal->m_pInherit );
@@ -1646,7 +1653,7 @@ bool SetConfigValueTyped(
 	}
 
 	// Call type-specific method to set it
-	if ( !AssignConfigValueTyped( pVal, eDataType, pArg ) )
+	if ( !AssignConfigValueTyped( &pVal->m_data, eDataType, pArg ) )
 		return false;
 
 	// Mark it as set
@@ -1736,6 +1743,29 @@ bool CSteamNetworkingUtils::SetConfigValue( ESteamNetworkingConfigValue eValue,
 		case k_ESteamNetworkingConfig_MTU_DataSize:
 			SpewWarning( "MTU_DataSize is readonly" );
 			return false;
+
+		case k_ESteamNetworkingConfig_ConnectionUserData:
+		{
+
+			// We only need special handling when modifying a connection
+			if ( eScopeType != k_ESteamNetworkingConfig_Connection )
+				break;
+
+			// Process the user argument, maybe performing type conversion
+			int64 newData;
+			if ( !AssignConfigValueTyped( &newData, eDataType, pValue ) )
+				return false;
+
+			// Lookup the connection
+			CSteamNetworkConnectionBase *pConn = GetConnectionByHandle( HSteamNetConnection( scopeObj ) );
+			if ( !pConn )
+				return false;
+
+			// Set the data, possibly fixing up existing queued messages, etc
+			pConn->SetUserData( pConn->m_connectionConfig.m_ConnectionUserData.m_data );
+			return true;
+		}
+
 	}
 
 	GlobalConfigValueEntry *pEntry = FindConfigValueEntry( eValue );

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.cpp

@@ -615,7 +615,6 @@ CSteamNetworkConnectionBase::CSteamNetworkConnectionBase( CSteamNetworkingSocket
 : m_pSteamNetworkingSocketsInterface( pSteamNetworkingSocketsInterface )
 {
 	m_hConnectionSelf = k_HSteamNetConnection_Invalid;
-	m_nUserData = -1;
 	m_eConnectionState = k_ESteamNetworkingConnectionState_None;
 	m_eConnectionWireState = k_ESteamNetworkingConnectionState_None;
 	m_usecWhenEnteredConnectionState = 0;
@@ -943,6 +942,9 @@ bool CSteamNetworkConnectionBase::BInitConnection( SteamNetworkingMicroseconds u
 		return false;
 	}
 
+	// Bind effective user data into the connection now.  It can no longer be inherited
+	m_connectionConfig.m_ConnectionUserData.Set( m_connectionConfig.m_ConnectionUserData.Get() );
+
 	// Make sure a description has been set for debugging purposes
 	SetDescription();
 
@@ -1698,7 +1700,7 @@ ESteamNetConnectionEnd CSteamNetworkConnectionBase::CheckRemoteCert( const CertA
 
 void CSteamNetworkConnectionBase::SetUserData( int64 nUserData )
 {
-	m_nUserData = nUserData;
+	m_connectionConfig.m_ConnectionUserData.Set( nUserData );
 
 	// Change user data on all messages that haven't been pulled out
 	// of the queue yet.  This way we don't expose the client to weird
@@ -1707,7 +1709,7 @@ void CSteamNetworkConnectionBase::SetUserData( int64 nUserData )
 	for ( CSteamNetworkingMessage *m = m_queueRecvMessages.m_pFirst ; m ; m = m->m_links.m_pNext )
 	{
 		Assert( m->m_conn == m_hConnectionSelf );
-		m->m_nConnUserData = m_nUserData;
+		m->m_nConnUserData = nUserData;
 	}
 }
 
@@ -1743,7 +1745,7 @@ void CSteamNetworkConnectionBase::ConnectionPopulateInfo( SteamNetConnectionInfo
 	info.m_eState = CollapseConnectionStateToAPIState( m_eConnectionState );
 	info.m_hListenSocket = m_pParentListenSocket ? m_pParentListenSocket->m_hListenSocketSelf : k_HSteamListenSocket_Invalid;
 	info.m_identityRemote = m_identityRemote;
-	info.m_nUserData = m_nUserData;
+	info.m_nUserData = GetUserData();
 	info.m_eEndReason = m_eEndReason;
 	V_strcpy_safe( info.m_szEndDebug, m_szEndDebug );
 	V_strcpy_safe( info.m_szConnectionDescription, m_szDescription );
@@ -3314,7 +3316,7 @@ int64 CSteamNetworkConnectionPipe::_APISendMessageToConnection( CSteamNetworking
 	pMsg->m_nMessageNumber = nMsgNum;
 	pMsg->m_conn = m_pPartner->m_hConnectionSelf;
 	pMsg->m_identityPeer = m_pPartner->m_identityRemote;
-	pMsg->m_nConnUserData = m_pPartner->m_nUserData;
+	pMsg->m_nConnUserData = m_pPartner->GetUserData();
 	pMsg->m_usecTimeReceived = usecNow;
 
 	// Pass directly to our partner

src/steamnetworkingsockets/clientlib/steamnetworkingsockets_connections.h

@@ -315,7 +315,12 @@ public:
 //
 
 	// Get/set user data
-	inline int64 GetUserData() const { return m_nUserData; }
+	inline int64 GetUserData() const
+	{
+		// User data is locked when we create a connection!
+		Assert( m_connectionConfig.m_ConnectionUserData.IsSet() );
+		return m_connectionConfig.m_ConnectionUserData.m_data;
+	}
 	void SetUserData( int64 nUserData );
 
 	// Get/set name
@@ -574,9 +579,6 @@ protected:
 	/// Called from BInitConnection, to start obtaining certs, etc
 	virtual void InitConnectionCrypto( SteamNetworkingMicroseconds usecNow );
 
-	/// User data
-	int64 m_nUserData;
-
 	/// Name assigned by app (for debugging)
 	char m_szAppName[ k_cchSteamNetworkingMaxConnectionDescription ];
 

src/steamnetworkingsockets/steamnetworkingsockets_internal.h

@@ -723,6 +723,7 @@ struct ConnectionConfig
 	ConfigValue<int32> m_Unencrypted;
 	ConfigValue<int32> m_SymmetricConnect;
 	ConfigValue<int32> m_LocalVirtualPort;
+	ConfigValue<int64> m_ConnectionUserData;
 
 	ConfigValue<int32> m_LogLevel_AckRTT;
 	ConfigValue<int32> m_LogLevel_PacketDecode;