Fix KeyNotFoundException by adding TryGetValue checks in EntityManager#67
Fix KeyNotFoundException by adding TryGetValue checks in EntityManager#67Kyr27 wants to merge 1 commit into
Conversation
…seconds of running the GameServer, but not entering the game. This error was due to using direct indexing in a few problematic areas. List of changes: - Replace direct indexing with TryGetValue in OnRemovedEntity - Add TryGetValue guard to FlushViewChangesToScoped - Replace direct indexing with TryGetValue in Tick. - Added TryGetValue guard to ScopedPlayersByEntity
|
Do you happen to know, why the index becomes invalid? Why can't the entity be found with the given id? |
Race conditions, concurrent dictionaries don't make you fully immune to them, so you still have to double check everything before touching any of it. There are probably other ways to fix it too, but this one is the simplest probably. For example, an entity could be removed from ScopedPlayersByEntity by the time we get it from Shard.Entities and when we try to access the dictionary. Adding a simple check like TryGetValue is the easiest way to prevent a crash. I added the checks wherever i felt like it could happen, it may be overkill, but otherwise it could become a problem later on, you never know with race conditions. |
The index can become invalid because entities in ScopedPlayersByEntity can be removed at any time, but others may still try to access it before the removal is fully completed. |
|
Hi, thanks for your contribution! I set the server to LoadZoneEntities in Zone 448, but I couldn't reproduce the issue after running the server over 5 minutes, several times. Do you have any more details for repro steps? Did you log into the server while running? Looking at the stacktrace, I suspect it it could be caused by AbilitySystem.ProcessTarget potentially processing an entity with more than one active effect, where one effect executes DestroyAbilityObjectCommand and removes the entity itself. Then when the next effect on the entity is processed, the entity has already been removed from the manager. Regarding the changes, I think it's a good idea to add guards to FlushViewChangesToScoped and SendToScoped. I'm not sure it's really needed for Tick and OnRemovedEntity but we could keep that for good measure. One thing I don't see is why it's also being applied to the usage of NetChannels though. That dict should be immutable. |
There aren't really any special steps to reproduce, everything was done as instructed:
The above was done on Windows 10 with Extended Security Updates. version 22H2 OS Build 19045.7184 At some point i tried getting into the game before it crashes, however the GameServer would always crash before i finished loading. The only real change i did at first was change the .NET version to .NET 10 of AeroMessages so that i didn't have to wait for VS to install .NET 6.0. That threw an exception so i downgraded to .NET 8.0 (i really didn't want to install another .NET version...), that threw an exception too, so i then downloaded .NET 6.0, that too, however, threw the same exception. I also tried running the 1.2.0 release, works as expected without any issues. About the changes you are probably right, i didn't pay that much attention and just wanted to get into the game, if NetChannels is immutable it doesn't need it. This was kind of a dirty fix honestly, it's probably better to add proper locks. I think this implementation is too laggy or something else is also happening, now i noticed that spawning anything causes a short connection problem to appear, although that may have been partially due to running Visual Studio Debugger in the background. Should i make a new issue and move all the relevant steps and logs over there? |
|
Here are the logs without adding guards, crashes after 1:08 min: Visual Studio Debugger catched: |
|
I moved all the details about the bug into a new issue. #68 (comment) I also tested this fix again and i think it's too laggy or there is something else also going on, spawning any kind of entity causes short connection problems. It may be better to consider alternative solutions. |
3 participants
Summary
Fix for the KeyNotFoundException in EntityManager that was happening after around a minute of running the GameServer, but not entering the game. This error was due to using direct indexing in a few problematic areas.
List of changes:
Error Details:
System.Collections.Generic.KeyNotFoundException
HResult=0x80131577
Message=The given key '2302054457216711424' was not present in the dictionary.
Source=System.Collections.Concurrent
StackTrace:
at System.Collections.Concurrent.ConcurrentDictionary`2.ThrowKeyNotFoundException(TKey key) in System.Collections.Concurrent\ConcurrentDictionary.cs:line 1124
at GameServer.Systems.EntityManager.EntityManager.FlushChanges(IEntity entity)
at GameServer.Entities.BaseAptitudeEntity.ClearEffect(EffectState state)
at GameServer.Aptitude.AbilitySystem.DoRemoveEffect(EffectState activeEffect)
at GameServer.Aptitude.AbilitySystem.ProcessTarget(IAptitudeTarget entity, UInt64 currentTime)
at GameServer.Aptitude.AbilitySystem.Tick(Double deltaTime, UInt64 currentTime, CancellationToken ct)
at GameServer.Shard.Tick(Double deltaTime, UInt64 currentTime, CancellationToken ct)
at GameServer.Shard.RunThread(CancellationToken ct)
at Shared.Udp.Utils.<>c__DisplayClass1_0.b__0(Object o)
at System.Threading.Thread.StartCallback()