Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove automatic (delayed) reseed-on-fork #1379

Merged
merged 9 commits into from
Mar 18, 2024

Conversation

dhardy
Copy link
Member

@dhardy dhardy commented Feb 7, 2024

See also #1362 #1377
CC @thomcc

Remove integrated fork protection.

Add ThreadRng::reseed and doc to suggest calling this on fork.

Make ReseedingRng::reseed also clear the random value cache.

@dhardy dhardy requested a review from newpavlov February 7, 2024 14:42
/// (every 64 kiB — see [`ReseedingRng`] documentation for details).
///
/// `ThreadRng` is not automatically reseeded on fork. It is recommended to
/// explicitly call [`ThreadRng::reseed`] immediately after a fork, for example:
Copy link
Member

@newpavlov newpavlov Feb 7, 2024

Choose a reason for hiding this comment

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

It also could be worth to mention pthread_atfork as a potential approach for doing this.

UPD: Further in the docs fork handlers are discussed and it's indeed not safe to mess with ThreadRng state in fork handlers, including child ones.

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically, it might be okay: ThreadRng methods should not be called at the time fork() happens. But I don't see the need to advertise this beyond what is already said.

@dhardy
Copy link
Member Author

dhardy commented Feb 7, 2024

Nightly builds fail due to zerocopy (google/zerocopy#844).

Probably a good idea to leave this PR open a little longer anyway in case anyone has concerns about the removal of automatic reseed-on-fork.

@dhardy dhardy changed the title Reseed on fork Remove automatic (delayed) reseed-on-fork Feb 7, 2024
@newpavlov
Copy link
Member

cc @tarcieri @briansmith

@dhardy dhardy mentioned this pull request Feb 13, 2024
23 tasks
@dhardy
Copy link
Member Author

dhardy commented Mar 18, 2024

No further comments, so I'll merge now.

In case @tarcieri @briansmith or others have concerns, we still have the option to revert before 0.9.

@dhardy dhardy merged commit 4cbbb34 into rust-random:master Mar 18, 2024
12 checks passed
@dhardy dhardy mentioned this pull request Apr 29, 2024
5 tasks
@dhardy dhardy deleted the reseed_on_fork branch July 8, 2024 10:03
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.

2 participants