Thread safe ID generation

[antonfirsov]

Fix #209 and use cryptographically strong random numbers Edit: removed the latter, the vulnerability doesn't apply to clients..

[antonfirsov]

Without the change the number of collisions is significantly higher.

[MichaCo]

I was actually adding a fix for this in my open PR, too:
https://github.com/MichaCo/DnsClient.NET/pull/207/files#diff-6ce9119d23f07ed09bd510d31b8f53054f0909b741bcfad887ab7b0d8aeba0dc

Now I'm wondering, why did you remove the version with RandomNumberGenerator in favor of a lock?
I do get roughly the same result with your unit test with my version at least ;)

        private static ushort GetRandom()
        {
            var block = new byte[2];
            s_generator.GetBytes(block);
            return BitConverter.ToUInt16(block, 0);
        }

[antonfirsov]

In #207 the two implementation paths have different characteristics: RandomNumberGenerator is cryptographically secure (but slower) while Random is not secure (but fast). I orginally used RandomNumberGenerator, but removed it in a2ee64c for simplicity. What made the code more complicated than ideal is the exlusion of id==1 which you don't do in #207 for old targets. I fail to understand why is id != 1 needed, but I wanted to keep the old logic.

In my understanding client txid-s don't have to be cryptographically secure random numbers, so no need to use RandomNumberGenerator unless DnsClient.NET is expected to be used in a recursive DNS server.

[MichaCo]

gotcha.

In my understanding client txid-s don't have to be cryptographically secure random numbers, so no need to use RandomNumberGenerator unless DnsClient.NET is expected to be used in a recursive DNS server.

yup that is also my understanding.