As noted by other commenters, it's not threadsafe in that two different threads might initialize the class at the same time. However, I don't see that anyone bothered to ask "So what?"
If two threads initialize it at once, they'll each create an instance. One will live for the life of the AppDomain. The other will get used briefly, then thrown away. It's a little messy, but will it cause any actual problems?
It's not holding any important state that must not be lost, and it's not holding on to any resources that might be leaked. The worst case is just that you initialize two RNGs instead of one, so you might get slightly different random numbers than you would otherwise get. But they're supposed to be crypto-secure numbers, not "repeatable random" like you might want for a saved game seed, so that seems like it should be acceptable. Unless there's some deep crypto reason that such a thing would be bad?
The two things that might actually matter are that the GeneratedBytesCount might be wrong, and if someone registered an event handler for the GenerateRandom256Pre event it might get lost. Neither of those things are actually used anywhere in the code, but it is possible that client code could use them.
Bottom line I'd say it's not great, but in the code that they actually have it will not cause any problems.
Aside: technically the class is not marked as threadsafe, but since it's a singleton it should probably be threadsafe anyway.
If two threads initialize it at once, they'll each create an instance. One will live for the life of the AppDomain. The other will get used briefly, then thrown away. It's a little messy, but will it cause any actual problems?
It's not holding any important state that must not be lost, and it's not holding on to any resources that might be leaked. The worst case is just that you initialize two RNGs instead of one, so you might get slightly different random numbers than you would otherwise get. But they're supposed to be crypto-secure numbers, not "repeatable random" like you might want for a saved game seed, so that seems like it should be acceptable. Unless there's some deep crypto reason that such a thing would be bad?
The two things that might actually matter are that the GeneratedBytesCount might be wrong, and if someone registered an event handler for the GenerateRandom256Pre event it might get lost. Neither of those things are actually used anywhere in the code, but it is possible that client code could use them.
Bottom line I'd say it's not great, but in the code that they actually have it will not cause any problems.
Aside: technically the class is not marked as threadsafe, but since it's a singleton it should probably be threadsafe anyway.