Bugfix: Make the Put on the SimpleDnsCache idempotent

[evertmulder]

Bugfix for the exception: itemwith the same key has already been added.
Make the Put on the SimpleDnsCache idempotent when a same host is
resolved.

[rogeralsing]

Is this really thread safe?
This seems to be used from a system extension (DnsExt : IOExtension) which makes me think it is a shared resource.
And the implementation is based on mutability, so shared mutable state unless I'm reading this wrong.

cc @akkadotnet/developers

[rogeralsing]

This is related but not part of the actual PR:

        internal void Put(Dns.Resolved r, long ttl)
        {
            var c = _cache.Value;
            if (!_cache.CompareAndSet(c, c.Put(r, ttl)))
                Put(r, ttl);
        }

the Put method use an atomic reference to compare and set.. but the values that are compared are the same instance, it's the same c that is compared to itself.

            public Cache Put(Dns.Resolved answer, long ttl)
            {
                var until = _clock() + ttl;
                _queue.Add(new ExpiryEntry(answer.Name, until));
                _cache.Add(answer.Name, new CacheEntry(answer, until));
                return this; //<----------------------- returns itself.
            }

c and c.Put(..) refers to the same thing, and thus the compare and set will always succeed.

The code consuming this is based on immutability and the SimpleDnsCache is based on mutability..

IMO, SimpleDnsCache needs to be rewritten or the consuming code will be subject to race conditions...

[rogeralsing]

The corresponding method in Scala looks like this:

    def put(answer: Resolved, ttlMillis: Long): Cache = {
      val until = clock() + ttlMillis

      new Cache(
        queue + new ExpiryEntry(answer.name, until),
        cache + (answer.name -> CacheEntry(answer, until)),
        clock)
    }

Which clearly returns a new instance of the cache.

[rogeralsing]

It seems that SimpleDnsManager is and actor and the only consumer of this extension. so it might be safe for now. but it is still flaky to have an extension that is not thread safe IMO.

[evertmulder]

Yes the code does not look Thread Safe and the implementation looks a bit different than the Scala version. Also the Datetime.Now should use the UtcNow I think. Perhaps some more people can have a look at this class.

[rogeralsing]

As this PR is better than the current code, I'm merging this.
But I will open an issue for a rewrite of the SimpleDnsCache to follow the Scala version.