Document thread safety of RandomNumberGenerator

[jasper-d]

Issue description I cannot find information w.r.t. the thread safety of members of System.Security.Cryptorgraphy.RandomNumberGenerator. Knowing if calls must be synchronized by the caller is important.

RNGCryptoServiceProvider.GetBytes is thread safe according to the documentation.

Target framework

• [x] .NET Core
• [x] .NET Framework
• [x] .NET Standard

[carlossanlop]

@bartonjs something we could improve in the Cryptography docs?

[bartonjs]

I don't know where or how we'd document it.

• RandomNumberGenerator.Fill, RandomNumberGenerator.GetBytes(int) RandomNumberGenerator.GetInt32(*) are all thread safe, because they're statics, and our default thread-safety statement says statics are thread-safe (though we don't seem to produce the default thread safety statement on the docs anymore?)
• On .NET Core: the instance methods of the instance produced by RandomNumberGenerator.Create() are thread-safe.
• Otherwise an instance of RandomNumberGenerator may or may not be thread-safe.

Consequences:

• If you accept a RandomNumberGenerator instance as a parameter you should assume it's not thread-safe.
• If you know you're on Core you can parallel-access a RandomNumberGenerator.Create() instance safely.
• If you know you have a RNGCryptoServiceProvider instance you can parallel-access it safely (since it's documented as thread-safe).

Recommended action:

• Use the static methods on TFMs where they're available.
• Don't assume RandomNumberGenerator instances are thread-safe (unless you can prove their origin).

[arielhammon]

@bartonjs

I don't know where or how we'd document it.

• RandomNumberGenerator.Fill, RandomNumberGenerator.GetBytes(int) RandomNumberGenerator.GetInt32(*) are all thread safe, because they're statics, and our default thread-safety statement says statics are thread-safe (though we don't seem to produce the default thread safety statement on the docs anymore?)
• On .NET Core: the instance methods of the instance produced by RandomNumberGenerator.Create() are thread-safe.
• Otherwise an instance of RandomNumberGenerator may or may not be thread-safe.

I've searched the documentation and source code for information about the thread-safety of static methods of RandomNumberGenerator. You say they are thread safe because they are statics. I can easily write a static method that is not thread safe, so my question is, are they thread safe because .Net authors ensure that ALL static methods are thread safe or are they thread safe for some other reason?

TIA.

[bartonjs]

The best place where this is written down that I can find at the moment is https://learn.microsoft.com/en-us/dotnet/standard/threading/managed-threading-best-practices#recommendations-for-class-libraries

• Make static data (Shared in Visual Basic) thread safe by default.
• Do not make instance data thread safe by default.

For everything that is in the shared runtime for .NET (formerly .NET Core), or for the GAC after a clean install of .NET Framework, the assumption is that every static member is thread-safe and every instance member is not... unless they're documented otherwise.

These principles should generally be followed by other library designers, but as you said, one can easily ignore the guidelines and make a static member that is not thread-safe.

ConcurrentDictionary, for example, has thread-safe instance members (except the explicit implementations of IDictionary). Since having thread-safe instance members is not the default, it has an explicit "Thread Safety" section in the docs: https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2?view=net-9.0#thread-safety

[arielhammon]

@bartonjs Thank-you. I will gladly accept this explanation, but I confess that I wish this policy were somehow made much more explicit and apparent in the documentation. I have many times encountered this concept (.Net statics are thread safe by default), but when I tried to find the explicit source, I couldn't.