From e027176162f974938beae2e0387171f27ee014d5 Mon Sep 17 00:00:00 2001 From: Luke Stampfli Date: Sat, 20 Feb 2021 00:33:04 +0000 Subject: [PATCH 1/4] fix: don't reuse bitstream in HandleIncomingData to prevent message corruption --- com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs index b677cc9854..8f588006a9 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs @@ -1041,7 +1041,6 @@ private void HandleRawTransportPoll(NetEventType eventType, ulong clientId, Chan } } - private readonly BitStream m_InputStreamWrapper = new BitStream(new byte[0]); private readonly RpcBatcher m_RpcBatcher = new RpcBatcher(); internal void HandleIncomingData(ulong clientId, Channel channel, ArraySegment data, float receiveTime, bool allowBuffer) @@ -1051,6 +1050,7 @@ internal void HandleIncomingData(ulong clientId, Channel channel, ArraySegment Date: Mon, 22 Feb 2021 09:06:35 +0000 Subject: [PATCH 2/4] fix: Fix prefix --- com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs index 8f588006a9..3013781a1d 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs @@ -1050,7 +1050,7 @@ internal void HandleIncomingData(ulong clientId, Channel channel, ArraySegment Date: Thu, 25 Feb 2021 18:53:41 +0000 Subject: [PATCH 3/4] fix: Add fallback input stream wrapper when handling buffered messages to fix a rare race condition bug. --- .../Runtime/Core/NetworkingManager.cs | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs index 1930e4c345..efe93bdad0 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs @@ -1029,6 +1029,11 @@ private void HandleRawTransportPoll(NetEventType eventType, ulong clientId, Chan } } + private readonly BitStream m_InputStreamWrapper = new BitStream(new byte[0]); + bool m_InputStreamWrapperUsed; + // The fallback wrapper is used in case we have to handle incoming data but the InputStreamWrapper is already being used. + // This change is needed because MLAPI calls HandleIncomingData nested when it is applying buffered messages to an object spawned in HandleIncomingData. + private readonly BitStream m_FallbackInputStreamWrapper = new BitStream(new byte[0]); private readonly RpcBatcher m_RpcBatcher = new RpcBatcher(); internal void HandleIncomingData(ulong clientId, Channel channel, ArraySegment data, float receiveTime, bool allowBuffer) @@ -1038,12 +1043,22 @@ internal void HandleIncomingData(ulong clientId, Channel channel, ArraySegment Date: Tue, 2 Mar 2021 10:48:33 +0000 Subject: [PATCH 4/4] docs: Add comment explaining why fallback wrapper works fine here --- .../Runtime/Core/NetworkingManager.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs index 9696bbdb95..95d2c5b6ee 100644 --- a/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs +++ b/com.unity.multiplayer.mlapi/Runtime/Core/NetworkingManager.cs @@ -1073,6 +1073,11 @@ private void HandleRawTransportPoll(NetEventType eventType, ulong clientId, Chan bool m_InputStreamWrapperUsed; // The fallback wrapper is used in case we have to handle incoming data but the InputStreamWrapper is already being used. // This change is needed because MLAPI calls HandleIncomingData nested when it is applying buffered messages to an object spawned in HandleIncomingData. + // This fallback wrapper solution works because HandleIncomingData will never get nested more then once because: + // - Messages we buffer and execute in nest level 1 can never end up in another HandleIncomingData call. This is true because HandleIncomingData is only called in two cases: + // 1. When a new message arrives (nest level 0) + // 2. When that new message spawns an object and applies buffered messages (nest level 1) + //Nest level 1 can never trigger case 1. or 2. again because case 1. can only be triggered by the server sending a spawn packet down and not locally and case 2. can only be triggered by case 1.. private readonly BitStream m_FallbackInputStreamWrapper = new BitStream(new byte[0]); private readonly RpcBatcher m_RpcBatcher = new RpcBatcher();