Skip to content

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

Merged
TimWolla merged 1 commit intophp:masterfrom
anthonyryan1:master
Mar 23, 2026
Merged

Add a warning about the subtle global state in legacy random functions#5414
TimWolla merged 1 commit intophp:masterfrom
anthonyryan1:master

Conversation

@anthonyryan1
Copy link
Copy Markdown
Contributor

@anthonyryan1 anthonyryan1 requested a review from TimWolla as a code owner March 7, 2026 03:07
Copy link
Copy Markdown
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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”.

@TimWolla TimWolla requested a review from Girgias March 9, 2026 21:16
@anthonyryan1 anthonyryan1 changed the title Add a warning about the footgun that is combining legacy MT random functions and pcntl_fork Add a warning about the subtle global state in legacy random functions Mar 10, 2026
@anthonyryan1 anthonyryan1 requested a review from Girgias March 10, 2026 18:36
@anthonyryan1
Copy link
Copy Markdown
Contributor Author

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.

@anthonyryan1 anthonyryan1 force-pushed the master branch 6 times, most recently from 066688d to 317d8dd Compare March 12, 2026 15:11
Copy link
Copy Markdown
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes for visibility of: #5414 (comment)

@TimWolla TimWolla merged commit d6dc2be into php:master Mar 23, 2026
2 checks passed
@TimWolla
Copy link
Copy Markdown
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pcntl_fork() should reseed the MT rand in all children

3 participants