WatsonTcpServer v6.0.3 throws ArgumentException when disposed: "An item with the same key has already been added"

[yallie]

Hello!

Not sure if it's relevant to issue #303, but sometimes I get another exception from the code below:

var server = new WatsonTcpServer("127.0.0.1", 9000);
server.Events.MessageReceived += (s, e) => Console.WriteLine("srecv");
server.Start();

var client = new WatsonTcpClient("127.0.0.1", 9000);
client.Events.MessageReceived += (s, e) => Console.WriteLine("crecv");
client.Connect();

await client.SendAsync("aaa");
await Task.Delay(100);
server.Dispose();

System.AggregateException : One or more errors occurred.
---- System.ArgumentException : An item with the same key has already been added. Key: be8924c2-0945-4b7f-be64-2996d62d9b56

Stack Trace: 
Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
Task.Wait()
WatsonTcpServer.Dispose(Boolean disposing)
WatsonTcpServer.Dispose()
RegressionTests.WatsonTcpThrowsCanceledExceptionOnDispose() line 29
<>c.<ThrowAsync>b__128_0(Object state)
----- Inner Stack Trace -----
Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
Dictionary`2.Add(TKey key, TValue value)
ClientMetadataManager.AddClientKicked(Guid guid)
WatsonTcpServer.DisconnectClientAsync(Guid guid, MessageStatus status, Boolean sendNotice, CancellationToken token)
WatsonTcpServer.DisconnectClientsAsync(MessageStatus status, Boolean sendNotice, CancellationToken token)

The error originates here: WatsonTcpServer.cs, line 572

if (!_ClientManager.ExistsClientTimedout(guid)) _ClientManager.AddClientKicked(guid);

The problem is introduced in this commit: NuGet v6.0.3, internal refactor e.g. ClientManager).

Older versions use ConcurrentDictionary instances for clients, last seen, kicked, etc:

private ConcurrentDictionary<Guid, DateTime> _UnauthenticatedClients = new ConcurrentDictionary<Guid, DateTime>();
private ConcurrentDictionary<Guid, ClientMetadata> _Clients = new ConcurrentDictionary<Guid, ClientMetadata>();
private ConcurrentDictionary<Guid, DateTime> _ClientsLastSeen = new ConcurrentDictionary<Guid, DateTime>();
private ConcurrentDictionary<Guid, DateTime> _ClientsKicked = new ConcurrentDictionary<Guid, DateTime>();
private ConcurrentDictionary<Guid, DateTime> _ClientsTimedout = new ConcurrentDictionary<Guid, DateTime>();

New refactored version use ordinal Dictionary instances protected by locks in ClientMetadataManager.cs:

private readonly object _ClientsKickedLock = new object();
private Dictionary<Guid, DateTime> _ClientsKicked = new Dictionary<Guid, DateTime>();
...
internal void AddClientKicked(Guid guid)
{
    lock (_ClientsKickedLock)
    {
        _ClientsKicked.Add(guid, DateTime.UtcNow);
    }
}

Older versions use _ClientsKicked.TryAdd which is thread-safe by design:

if (!_ClientsTimedout.ContainsKey(guid)) _ClientsKicked.TryAdd(guid, DateTime.UtcNow);

Can we revert it back to ConcurrentDictionary?

[yallie]

Occurs from time to time in our unit tests here on Github Actions
since we updated WatsonTcp dependency to the latest version:

[jchristn]

Added checks for this inside of lock statements within ClientMetadataManager, will post 6.0.8 in a few moments with the fix.