change uuid to use Random.RandomDevice() instead of Random.default_rng()

[ssikdar1]

Description

Fixes #35860

Change Random.default_rng() in the UUID functions to Random.RandomDevice()

Testing

Ran $ ./julia stdlib/UUIDs/test/runtests.jl with no issues

[StefanKarpinski]

Looks good to me. A simple test that could be done after this is setting the global RNG seed to the same value twice and generating a new UUID by each of these methods after and checking that they are not the same. Before this, that would have failed, so that's a good regression test.

[rfourquet]

Also, this needs docs and news. And maybe a "!!! compat" annotation saying that this new behavior starts in version 1.6.

[JeffBezanson]

This may be the right decision but I believe RandomDevice can be reeeally slow --- are we sure this is ok?

[tkf]

(pinging @Keno who had the same concern #32954 (comment))

I think the default should be the safe one. If you know you need tons of random UUIDs, I think you can always swap the RNG argument. But warning this in the docstring would be nice.

[rfourquet]

I'm not totally sold on the idea either. There is something nice in knowing that generally all randomness comes from the default_rng or explicit RNGs and reproducibility can be achieved with seed!. Generally, the default one is seeded only in tests, and I would tend to consider it bad practice when it's seeded in a package (not in tests). So using RandomDevice() here goes in the way of the user who needs reproducibility (for whatever reason, e.g. debugging). On the other hand, it's easy for applications to explicitly use RandomDevice() for UUIDs. Using RandomDevice() here is a special case and opens the door to more special cases, and special cases increase complexity.

[StefanKarpinski]

I think the danger of producing the same UUIDs repeatedly when they are supposed to be universally unique is way too high. This is definitely one of those things that has caused problems because people (myself included) just don't realize when they call seed! that they will cause the same UUIDs to be generated repeatedly. Super dangerous. Especially because of the decision which I don't think we can roll back at this point to automatically call seed! at the start of each testset. The other case that's also really bad is using a default RNG for generating names of temp files, but I think that has now been fixed?

[StefanKarpinski]

Combined with the fact that UUIDs are sometimes relevant for security, this seems good to me.

[JeffBezanson]

How about using a dedicated default RNG for UUIDs that's initialized from RandomDevice at startup?

[StefanKarpinski]

I think that would be fine. It might be good to do something that is more secure as well, e.g. initialize a seed from RandomDevice on the first call and then do hash-based UUID generation combining that seed value with a counter using SHA256 or something like that.

[StefanKarpinski]

I feel like this is pretty simple and we can just do this and see if anyone actually cares about the performance of generating UUIDs. If they do, then we can consider something more complicated.

[rfourquet]

Why did you make this change? Creating a new RandomDevice() is extremely cheap on non-Windows, and on Windows having a global uuid_rng is not thread-safe...

[ssikdar1]

@rfourquet I guess misunderstood @JeffBezanson 's comment above?

How about using a dedicated default RNG for UUIDs that's initialized from RandomDevice at startup?

[rfourquet]

My interpretation of it was using an RNG like default_rng() (currently MersenneTwister() which is thread-safely initialized), which is therefore immune against the unqualified Random.seed!() but way faster than RandomDevice(), but "initialized from RandomDevice()" means it's seeded from RandomDevice() at startup.

[rfourquet]

But Stefan's last comment suggests to keep it how it was, because the poor performance of RandomDevice might considered as a non-problem until someone complains.

[ssikdar1]

Okay! I reverted 507b454 . I'll leave it alone for now until there's something I should explicitly add/change/remove.

[tkf]

Since it is possible that the default RNG to be changed, I think it'd be better to avoid putting Random.RandomDevice() in the signature. Also, it makes future changes easier if we only describe the behavior and not the implementation. How about something like this?

uuid4([rng::AbstractRNG]) -> UUID

Generates a version 4 (random or pseudo-random) universally unique identifier (UUID), as specified by RFC 4122.

The default rng used by uuid4 is not GLOBAL_RNG and every invocation of uuid4() without an argument should be expected to return a unique identifier. Importantly, the outputs of uuid4 do not repeat even when Random.seed!(seed) is called. Currently (as of Julia 1.6), uuid4 uses Random.RandomDevice as the default rng. However, this is an implementation detail that may change in the future.

compat "Julia 1.6"
The output of uuid4 does not dependent on GLOBAL_RNG as of Julia 1.6.

[c42f]

This looks great to me — simple and safe, with the side benefit that it doesn't pull the complexity of all of Random to the default implementation of uuid4 (which helps greatly in #35894).

Any objections to merging and seeing how it turns out?