Atomicity bug and bad performance in AddressTerminatedTopic

[petrikero]

The AddressTerminatedTopic implementation of atomic updates for the _subscribers HashSet is incorrect and loses data when multiple updates are happening at the same time.

The bug is in the code lines below (from AddressTerminatedTopic.Subscribe() -- Unsubscribe() has similar problem). The issue is that the type of 'current' is actually the AtomicReference<>, not the HashSet<>, which means that the 'current' parameter passed into CompareAndSet() is implicitly converted to 'current.Value', which causes another atomic read of the value (which doesn't necessarily have the newly added element).

var current = _subscribers;
if (!_subscribers.CompareAndSet(current, new HashSet<IActorRef>(current.Value) {subscriber}))

The simple fix would be:

var current = _subscribers.Value;
if (!_subscribers.CompareAndSet(current, new HashSet<IActorRef>(current) {subscriber}))

This, however, still leaves a significant performance problem. The original Akka implementation uses a persistent data structure, which has an efficient implementation for single-element mutation, whereas this code does a full copy of the _subscribers HashSet (or even multiple copies in case of congestion). We saw this causing major problems due to lots of memory being allocated and garbage collect immediately afterward.

I'll send a PR where both of these issues are fixed by using a simple HashSet with locks to protect against data races.

I'm using Akka.Net version 1.3.17. The bug occurs on both Windows and dockerized Linux. I don't have simple repro steps, but the issue should be obvious when looking at the code carefully.

[petrikero]

Here's a link to the PR: #4307

[Aaronontheweb]

@petrikero included this into Akka.NET v1.3.18: https://github.com/akkadotnet/akka.net/releases/tag/1.3.18