Skip to content

Commit 024f67a

Browse files
authored
fix: Custom messaging manager exception (#3417)
<!-- Replace this block with what this PR does and why. Describe what you'd like reviewers to know, how you applied the engineering principles, and any interesting tradeoffs made. Delete bullet points below that don't apply, and update the changelog section as appropriate. --> <!-- Add short version of the JIRA ticket to the PR title (e.g. "feat: new shiny feature [MTT-123]") --> [MTTB-1230](https://jira.unity3d.com/browse/MTTB-1230) fixes: #3414 ## Changelog - Fixed: Unregistering a message handler inside the message handler no longer throws an exception. ## Testing and Documentation - Includes a unit test. <!-- Uncomment and mark items off with a * if this PR deprecates any API: ### Deprecated API - [ ] An `[Obsolete]` attribute was added along with a `(RemovedAfter yyyy-mm-dd)` entry. - [ ] An [api updater] was added. - [ ] Deprecation of the API is explained in the CHANGELOG. - [ ] The users can understand why this API was removed and what they should use instead. --> ## Backport <!-- If this is a backport: - Add the following to the PR title: "\[Backport\] ..." . - Link to the original PR. If this needs a backport - state this here If a backport is not needed please provide the reason why. If the "Backports" section is not present it will lead to a CI test failure. --> Backported by #3420
1 parent 10ef2eb commit 024f67a

File tree

3 files changed

+46
-4
lines changed

3 files changed

+46
-4
lines changed

com.unity.netcode.gameobjects/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
1313

1414
### Fixed
1515

16+
- Fixed an exception being thrown when unregistering a custom message handler from within the registered callback. (#3417)
1617

1718
### Changed
1819

com.unity.netcode.gameobjects/Runtime/Messaging/CustomMessageManager.cs

+12-4
Original file line numberDiff line numberDiff line change
@@ -180,14 +180,18 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, FastBufferReader read
180180
// We dont know what size to use. Try every (more collision prone)
181181
if (m_NamedMessageHandlers32.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler32))
182182
{
183+
// handler can remove itself, cache the name for metrics
184+
var messageName = m_MessageHandlerNameLookup32[hash];
183185
messageHandler32(sender, reader);
184-
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup32[hash], bytesCount);
186+
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, messageName, bytesCount);
185187
}
186188

187189
if (m_NamedMessageHandlers64.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler64))
188190
{
191+
// handler can remove itself, cache the name for metrics
192+
var messageName = m_MessageHandlerNameLookup64[hash];
189193
messageHandler64(sender, reader);
190-
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup64[hash], bytesCount);
194+
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, messageName, bytesCount);
191195
}
192196
}
193197
else
@@ -198,15 +202,19 @@ internal void InvokeNamedMessage(ulong hash, ulong sender, FastBufferReader read
198202
case HashSize.VarIntFourBytes:
199203
if (m_NamedMessageHandlers32.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler32))
200204
{
205+
// handler can remove itself, cache the name for metrics
206+
var messageName = m_MessageHandlerNameLookup32[hash];
201207
messageHandler32(sender, reader);
202-
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup32[hash], bytesCount);
208+
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, messageName, bytesCount);
203209
}
204210
break;
205211
case HashSize.VarIntEightBytes:
206212
if (m_NamedMessageHandlers64.TryGetValue(hash, out HandleNamedMessageDelegate messageHandler64))
207213
{
214+
// handler can remove itself, cache the name for metrics
215+
var messageName = m_MessageHandlerNameLookup64[hash];
208216
messageHandler64(sender, reader);
209-
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, m_MessageHandlerNameLookup64[hash], bytesCount);
217+
m_NetworkManager.NetworkMetrics.TrackNamedMessageReceived(sender, messageName, bytesCount);
210218
}
211219
break;
212220
}

com.unity.netcode.gameobjects/Tests/Runtime/Messaging/NamedMessageTests.cs

+33
Original file line numberDiff line numberDiff line change
@@ -279,5 +279,38 @@ public unsafe void ErrorMessageIsPrintedWhenAttemptingToSendNamedMessageWithTooB
279279
Assert.IsTrue(message.Contains($"Given message size ({msgSize} bytes) is greater than the maximum"), $"Unexpected exception: {message}");
280280
}
281281
}
282+
283+
[Test]
284+
public void NamedMessageHandlerIsUnregisteredWithoutException()
285+
{
286+
var messageName = Guid.NewGuid().ToString();
287+
const int numMessagesToSend = 3;
288+
const int expectedMessageHandlerCallCount = 1;
289+
290+
var messageHandlerCalled = 0;
291+
m_ServerNetworkManager.CustomMessagingManager.RegisterNamedMessageHandler(
292+
messageName,
293+
(_, _) =>
294+
{
295+
messageHandlerCalled++;
296+
m_ServerNetworkManager.CustomMessagingManager.UnregisterNamedMessageHandler(messageName);
297+
});
298+
299+
var messageContent = new ForceNetworkSerializeByMemcpy<Guid>(Guid.NewGuid());
300+
var writer = new FastBufferWriter(1300, Allocator.Temp);
301+
using (writer)
302+
{
303+
writer.WriteValueSafe(messageContent);
304+
for (var i = 0; i < numMessagesToSend; i++)
305+
{
306+
m_ServerNetworkManager.CustomMessagingManager.SendNamedMessage(
307+
messageName,
308+
m_ServerNetworkManager.LocalClientId,
309+
writer);
310+
}
311+
}
312+
313+
Assert.AreEqual(expectedMessageHandlerCallCount, messageHandlerCalled);
314+
}
282315
}
283316
}

0 commit comments

Comments
 (0)