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

Document that the random module doesn't support fork #74237

Closed
vstinner opened this issue Apr 12, 2017 · 14 comments
Closed

Document that the random module doesn't support fork #74237

vstinner opened this issue Apr 12, 2017 · 14 comments
Assignees
Labels
3.7 (EOL) end of life docs Documentation in the Doc dir

Comments

@vstinner
Copy link
Member

BPO 30051
Nosy @rhettinger, @pitrou, @vstinner, @serhiy-storchaka

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = 'https://github.com/rhettinger'
closed_at = <Date 2017-08-11.00:11:47.096>
created_at = <Date 2017-04-12.09:24:49.758>
labels = ['3.7', 'docs']
title = "Document that the random module doesn't support fork"
updated_at = <Date 2017-08-11.00:11:47.095>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2017-08-11.00:11:47.095>
actor = 'vstinner'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2017-08-11.00:11:47.096>
closer = 'vstinner'
components = ['Documentation']
creation = <Date 2017-04-12.09:24:49.758>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 30051
keywords = []
message_count = 14.0
messages = ['291534', '291537', '291540', '291541', '291544', '291773', '291788', '291797', '295889', '295894', '295896', '295897', '295898', '300146']
nosy_count = 5.0
nosy_names = ['rhettinger', 'pitrou', 'vstinner', 'docs@python', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue30051'
versions = ['Python 3.7']

@vstinner
Copy link
Member Author

When reviewing the issue bpo-30030, it reminded me that the random module "doesn't support fork": after fork, the parent and the child produce the same "random" number sequence.

I suggest to add a quick note about that: not a warning, just a note, as a reminder.

There is an exception: SystemRandom produces a different sequence after fork, since it uses os.urandom() (which runs in the kernel).

I am tempted to propose a solution in the note like using SystemRandom, but I'm not sure that it's a good idea. Some users may misunderstood the note and always use SystemRandom where random.Random is just fine for their needs.

Proposed note:

The random module doesn't support fork: the parent and the child process will produce the same number sequence.

Proposed solution:

If your code uses os.fork(), a workaround is to check if os.getpid() changes and in that case, create a new Random instance or reseed the RNG in the child process.

--

The tempfile module reminds the pid and instanciates a new RNG on fork.

Another option is to not add a note, but implement the workaround directly into the random module. But I don't think that it's worth it. Forking is a rare usecase, and calling os.getpid() may slowdown the random module. (I don't recall if os.getpid() requires a syscall or not, I'm quite sure that it's optimized at least on Linux to avoid a real syscall.)

@vstinner vstinner added 3.7 (EOL) end of life docs Documentation in the Doc dir labels Apr 12, 2017
@pitrou
Copy link
Member

pitrou commented Apr 12, 2017

I'm not sure that's necessary. fork() is a low-level primitive, people can/should use multiprocessing.Process instead, which does re-seed the PRNG.

@vstinner
Copy link
Member Author

I'm not sure that's necessary.

Do you mean that adding a note is not necessary? Or proposing a
solution in the note?

@pitrou
Copy link
Member

pitrou commented Apr 12, 2017

I mean mentioning fork(), which people usually don't call directly, is not necessary, so, indeed, a note isn't necessary.

@serhiy-storchaka
Copy link
Member

I think it is worth documenting.

Even if multiprocessing reseeds the module-global random generator, it doesn't reseed other instances of Random.

@rhettinger rhettinger assigned rhettinger and unassigned docspython Apr 17, 2017
@rhettinger
Copy link
Contributor

Several thoughts:

  • AFAICT, in entire the history of this module, no user has ever reported this as being a source of confusion, so I don't think there is a real documentation issue here.

  • Regarding, "after fork, the parent and the child produce the same "random" number sequence", this is the expected behavior of a PRNG. If it did something thing different, THAT would be a bug.

  • There random module is likely the wrong place for a note. It is more properly a topic about forking itself. Perhaps there is room for a FAQ entry about forking elaborating on the broad range of state that is shared across forks (lock and file descriptors, etc).

  • For those who need it, the API already supports reseeding and a way to make new instances of Random. Both of those are the standard ways of doing it for people who need independent generators in different threads.

@serhiy-storchaka
Copy link
Member

os.fork() documentation contains a warning that refers to ssl documentation where the use of OpenSSL’s internal random number generator in forked processes is documented more detailed. I think the same should be added for the random module.

@rhettinger
Copy link
Contributor

os.fork() documentation contains a warning that refers to ssl
documentation where the use of OpenSSL’s internal random number
generator in forked processes is documented more detailed.
I think the same should be added for the random module.

+1 It is reasonable to extend the docs for os.fork().

@pitrou
Copy link
Member

pitrou commented Jun 13, 2017

Victor, this issue should be obsolete now that the global Random instance is reseeded after fork().

@serhiy-storchaka
Copy link
Member

Even if the global Random instance is reseeded after fork(), other instances are not reseeded.

@vstinner
Copy link
Member Author

At least, it would be nice to document it for Python 3.6 and older.

@pitrou
Copy link
Member

pitrou commented Jun 13, 2017

This is true, but it's by design. If you want a specific random sequence (which is the primary use case for specific instances), you don't want fork() to mess with the random sequence.

@serhiy-storchaka
Copy link
Member

Yes, this is a feature, not a bug. But I think it is worth be explicitly documented.

@vstinner
Copy link
Member Author

Since Raymond and Antoine don't like the documentation idea, I just close my issue.

Hopefully, Python 3.7 now reseeds automatically random at fork!

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.7 (EOL) end of life docs Documentation in the Doc dir
Projects
None yet
Development

No branches or pull requests

4 participants