ext/pcntl: Children should not inherit random seeds

[anthonyryan1]

Fix GH-21351

[TimWolla]

It is a reasonable user expectation that the state of both processes after forking is identical and that includes the seed for the global Mt19937 instance - at least when it was manually seeded (which we don't currently track). I don't see much value in tracking that, since relying on a seedable global RNG is rather questionable in the first place.

Use random_int() (if you need secure random numbers) or a manually created Random\Engine combined with Random\Randomizer::getInt() (if you need control over your randomness) instead. Xoshiro256StarStar::jump() might come in handy for “reseeding” after a fork if secure randomness is not required.

[TimWolla]

Request changes for visibility.

[anthonyryan1]

I do agree that it's also reasonable to expect the global state to carry over into forked processes, there's precedent for that.

The part where it starts to feel odd, and made me open this is because of the lazy initialization of the MT rand seed leading to two radically different behaviors based on seemingly unrelated code.

If MT rand was initialized on script startup, rather than first use, forked children would always have the same seed. Use of MT rand within fork children would produce the same sequence 100% of the time. No matter whether MT rand was used prior to forking or not.

I'm aware of the newer randomizer, and use it everywhere that's crypto sensitive, but this code is not a crypto sensitive task, just array_rand() use.

Would you like it better if I amended this pull request so that the MT rand seed is always initialized prior to forking, so that every fork child has identical seed? We can still have consistency that way, and make sure nobody's relying on fragile behavior that could change abruptly.

[TimWolla]

The part where it starts to feel odd, and made me open this is because of the lazy initialization of the MT rand seed leading to two radically different behaviors based on seemingly unrelated code.

Yes, that's fair. Normally the lazy initialization should not lead to an observable behavioral difference, but in case of forking that is different.

I'm aware of the newer randomizer, and use it everywhere that's crypto sensitive, but this code is not a crypto sensitive task, just array_rand() use.

The new API is always better, even outside of cryptographic use (except possibly for verbosity in usage). I'd say in particularly outside of cryptograhic use, since for the latter you are mainly interested in random_bytes() rather than the randomization methods provided by the Randomizer.

The new object-oriented engines provide better-quality randomness than Mt19337, are faster and are not affected by the "global state" issue.

Would you like it better if I amended this pull request so that the MT rand seed is always initialized prior to forking, so that every fork child has identical seed? We can still have consistency that way, and make sure nobody's relying on fragile behavior that could change abruptly.

That would be better, yes. But I would still feel a little uneasy about changing the behavior of an API that IMO should only be used for backwards compatibility reasons, since folks still using this kind of API are particularly sensitive to behavioral changes. My solution of choice would thus be making this a doc-only change and recommending folks to migrate to the new API if they need finer-grained control over the behavior.