Add a warning about the subtle global state in legacy random functions

[anthonyryan1]

Fixes php/php-src#21351
Replaces php/php-src#21352

[TimWolla]

Thank you. I need to think about this a little more. Perhaps it's best to also have a generic “global state bad” message on the affected functions and just mention the fork gotcha in an “aside”.

[anthonyryan1]

I've refocused this on simply being a warning on the legacy random pages. Let me know if you'd like to see any other changes or improvements.

I've left pcntl out entirely, because arguably this problem is just that these functions still exist and the flaw is their use, rather than any overlap with pcntl.

[TimWolla]

I like the direction. Here's a suggestion that I feel is a little more precise (and hopefully not much more complicated to understand):

This function uses the global Mt19937 (“Mersenne Twister”) instance as the source of randomness and thus shares its state with all other functions using the global Mt19937. Using any of these functions affects the output of all the other functions, regardless of scope.

In particular seeding the Mt19937 instance with a fixed value using <function>mt_srand</function> or <function>srand</function> to generate a repeatable output sequence affects <emph>all</emph> following calls, unless the instance is reseeded with a random seed afterwards.

And of course, the existing warning that there are only 2^32 seeds applies …

[anthonyryan1]

Good suggestion!

I tried my best to incorporate your key points while focusing it a bit more on specific examples of the repeatable sequences changing.

The new iteration feels like it affects consumers of the MT global state, rather than seeders, so I removed the warning from mt_srand() and srand().

I also think with the caution refocused on repeatable sequences, I'm less certain it's needed on mt_rand() / rand() where that's a deliberate behavior people might rely upon.

So we've got a more specific warning, affecting fewer functions: array_rand, str_shuffle, shuffle where they advertise that they could suddenly become repeatable from global state set elsewhere.

Let me know if you'd like to see further iterations!

[TimWolla]

I also think with the caution refocused on repeatable sequences, I'm less certain it's needed on mt_rand() / rand() where that's a deliberate behavior people might rely upon.

I think the warning should also be listed for mt_rand() and rand() for completeness. While it's fairly obvious that they are affected, the global state issue is nevertheless real for them (e.g. by a library adding new calls internally) and the recommendation to use the Randomizer also applies.

Other than that, I'm happy with your phrasing. Thank you!

[TimWolla]

Request changes for visibility of: #5414 (comment)