I don't think this is the best approach. If you check https://en.cppreference.com/w/cpp/numeric/random/random_device, there's a note:

note: demo only: the performance of many
implementations of random_device degrades sharply
once the entropy pool is exhausted. For practical use
random_device is generally only used to seed
a PRNG such as mt19937

So in our case we need to construct an instance of mt19937 generator, seed it with the result of random_device and store as an instance variable. The current approach might have serious performance issues when the entropy is depleted

Moved generation code to the generateUUID function in Tools, using the suggested approach to generate it using seeded mt19937 and make them thread_local static.

It is technically a collision, but not something which we should worry about. If you think we need to log it, I would at least delete warning.

Yes, but we definitely need to know this in server logs. If these collisions will repeatedly occur that will indicate that something is wrong or needs to be improved in a random generation.

Use boost::uuids::to_string here instead. https://www.boost.org/doc/libs/1_78_0/libs/uuid/doc/uuid.html#to_string

It's more clear IMO.

Yeah, just drop the mt19937 generator.

std::random_device generator;

Making it thread-local might help performance a little bit so it keeps the same file descriptor.

We should probably just leave this as-is, and undo the changes

I think we should stay with the generateUUID() as a function in Tools. The function code is small but it will be reused in different places. Now we used it once in sessions, but I plan to use it to generate message IDs, and maybe in future updates too. In case we want to change something in the generator it would be much easier to refactor and don't repeat the same code right now.

Use boost::uuids::to_string here instead. https://www.boost.org/doc/libs/1_78_0/libs/uuid/doc/uuid.html#to_string

It's more clear IMO.

Already used it on the updates, find it after the first update, but thanks anyway ;) we don't need the cast here.

We should probably switch to using something that sources from /dev/urandom here too?

Yeah, just drop the mt19937 generator.

All that makes sense to me so switched to std::random_device here.

We should probably use just thread_local to be consistent with the codebase. Also in https://en.cppreference.com/w/cpp/language/storage_duration it is said that

It can be combined with static or extern to specify internal or external linkage (except for static data members which always have external linkage), respectively, but that additional static doesn't affect the storage duration.

So we don't need to specify the static.

Switched to thread_local only.

I say skip the database lookup and the while loop. Since the UUID is truly random now, a collision won't happen.

I say skip the database lookup and the while loop. Since the UUID is truly random now, a collision won't happen.

That makes sense. I've removed the DB fetch.