[issue16500] Add an 'atfork' module

[Antoine Pitrou]

Antoine Pitrou added the comment:

I've posted https://github.com/python/cpython/pull/1715 which adds a single 
`os.register_at_fork` function to allow registering at-fork handlers.  Comments 
welcome.

--
stage:  -> patch review
versions: +Python 3.7 -Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
pull_requests: +1805

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[A. Jesse Jiryu Davis]

Changes by A. Jesse Jiryu Davis :


--
nosy: +emptysquare

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Марк Коренберг]

Марк Коренберг added the comment:

Also I have to add, maybe you also add automatic file descriptor closing on 
fork. We already have O_CLOEXEC, we have no O_CLOFORK. So, maybe it useful to 
add such flag ?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Марк Коренберг]

Changes by Марк Коренберг :


--
nosy: +mmarkk

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Robert Cope]

Changes by Robert Cope :


--
nosy: +rpcope1

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Ionel Cristian Mărieș]

Changes by Ionel Cristian Mărieș :


--
nosy: +ionelmc

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Yury Selivanov]

Changes by Yury Selivanov yseliva...@gmail.com:


--
nosy: +yselivanov

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[xupeng]

Changes by xupeng xup...@xupeng.me:


--
nosy: +xupeng

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
versions: +Python 3.5 -Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Richard Oudkerk]

Richard Oudkerk added the comment:

Given PEP 446 (fds are now CLOEXEC by default) I prepared an updated patch 
where the fork lock is undocumented and subprocess no longer uses the fork 
lock.  (I did not want to encourage the mixing of threads with fork() without 
exec() by exposing the fork lock just for that case.)

But I found that a test for the leaking of fds to a subprocess started with 
closefds=False was somewhat regularly failing because the creation of CLOEXEC 
pipe fds is not atomic -- the GIL is not held while calling pipe().

It seems that PEP 446 does not really make the fork lock redundant for 
processes started using fork+exec.

So now I don't know whether the fork lock should be made public.  Thoughts?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Charles-François Natali]

Charles-François Natali added the comment:

 Given PEP 446 (fds are now CLOEXEC by default) I prepared an updated patch 
 where the fork lock is undocumented and subprocess no longer uses the fork 
 lock.  (I did not want to encourage the mixing of threads with fork() without 
 exec() by exposing the fork lock just for that case.)

 But I found that a test for the leaking of fds to a subprocess started with 
 closefds=False was somewhat regularly failing because the creation of CLOEXEC 
 pipe fds is not atomic -- the GIL is not held while calling pipe().

Was it on Linux? If yes, was it on an old kernel/libc? (I just want to
make sure that pipe2() is indeed used if available).

 It seems that PEP 446 does not really make the fork lock redundant for 
 processes started using fork+exec.

 So now I don't know whether the fork lock should be made public.  Thoughts?

IMO it should be kept private for now: I think it's really only useful
in some corner cases, and if it turns out to be useful, we can expose
it later.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[STINNER Victor]

STINNER Victor added the comment:

The PEP 446 does not offer any warranty on the atomicity on clearing the
inheritable flag.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Richard Oudkerk]

Richard Oudkerk added the comment:

It is a recent kernel and does support pipe2().

After some debugging it appears that a pipe handle created in Popen.__init__() 
was being leaked to a forked process, preventing Popen.__init__() from 
completing before the forked process did.

Previously the test passed because Popen.__init__() acquired the fork lock.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Christian Heimes]

Christian Heimes added the comment:

Richard, do you have time to get your patch ready for 3.4?

--
assignee: christian.heimes - 

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Richard Oudkerk]

Richard Oudkerk added the comment:

 Richard, do you have time to get your patch ready for 3.4?

Yes.  But we don't seem to have concensus on how to handle exceptions.  The 
main question is whether a failed prepare callback should prevent the fork from 
happenning, or just be printed.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Dwayne Litzenberger]

Dwayne Litzenberger added the comment:

 The main question is whether a failed prepare callback should prevent the 
 fork from happenning

Yes, I think an exception should prevent the fork from happening.

It's fail-safe for the PRNG case (you can guarantee that a fork won't occur 
without properly re-seeding a PRNG), and it makes bugs easier to catch in unit 
testing.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Christian Heimes]

Christian Heimes added the comment:

+1 for exception prevents fork

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Charles-François Natali]

Charles-François Natali added the comment:

I have a couple random remarks:
- now that FDs are non-inheritable by default, fork locks around
subprocess and multiprocessing shouldn't be necessary anymore? What
other use cases does the fork-lock have?
- the current implementation keeps hard-references to the functions
passed: so if one isn't careful, you can end up easily with a lot of
objects kept alive just because of those references, which can be a
problem
- also, since we're not sure about the API, and it's mostly intended
to be used for the interpreter/stdlib, how about making it private for
now, or at least provisional' (I think that's the term)?
- I'm also +1 on exceptions in prepare hook preventing fork, but we'll
need to play a bit with actual fork hooks to see if that's a
reasonable approach

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[STINNER Victor]

STINNER Victor added the comment:

now that FDs are non-inheritable by default, fork locks around
subprocess and multiprocessing shouldn't be necessary anymore? What
other use cases does the fork-lock have?

Well, on Windows, it's still not possible to inherit only one handle. If you 
mark temporary the handle as inheritable (os.set_handle_inheritable), it might 
be inherited by a diffrent child process if another Python thread spawn a 
process... It's probably unlikely, so it's one of the use case of such lock :-)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Richard Oudkerk]

Richard Oudkerk added the comment:

 - now that FDs are non-inheritable by default, fork locks around
 subprocess and multiprocessing shouldn't be necessary anymore? What
 other use cases does the fork-lock have?

CLOEXEC fds will still be inherited by forked children.

 - the current implementation keeps hard-references to the functions
 passed: so if one isn't careful, you can end up easily with a lot of
 objects kept alive just because of those references, which can be a
 problem

True, but you could make the same complaint about atexit.register().

One can fairly easily create something like weakref.finalize which uses atfork 
but is smart about not creating hard refs.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Charles-François Natali]

Charles-François Natali added the comment:

 Richard Oudkerk added the comment:

 - now that FDs are non-inheritable by default, fork locks around
 subprocess and multiprocessing shouldn't be necessary anymore? What
 other use cases does the fork-lock have?

 CLOEXEC fds will still be inherited by forked children.

Hum, right, I was thinking only about subprocess-created children
(where an exec follows immediately).

 - the current implementation keeps hard-references to the functions
 passed: so if one isn't careful, you can end up easily with a lot of
 objects kept alive just because of those references, which can be a
 problem

 True, but you could make the same complaint about atexit.register().

Yeah, but atexit is usually used for process-lifetime resources (I
mean, there's 'exit' in the name).
One of the main use cases for atfork hooks would be the numerous
stdlib objects which have locks (or locks themselves): most of such
objects have arbitrary lifetimes (e.g. logging, events, open files,
etc).

The risk of leak is IMO much greater.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Antoine Pitrou]

Antoine Pitrou added the comment:

 One of the main use cases for atfork hooks would be the numerous
stdlib objects which have locks (or locks themselves): most of such
objects have arbitrary lifetimes (e.g. logging, events, open files,
etc).
 The risk of leak is IMO much greater.

Well it is customary for callback-based APIs to hold strong references to their 
callbacks. If a library wants to avoid leaks, it should register a single 
callback which will then walk the current live resources and protect them.
(i.e. the resource lifetime issue should be solved by library or application 
code, not by the atfork module)

By the way, +0 to raising and aborting the fork when there's an exception. The 
only annoyance I can think about is a buggy library which would prevent the 
user from forking.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Charles-François Natali]

Charles-François Natali added the comment:

 Well it is customary for callback-based APIs to hold strong references to 
 their callbacks. If a library wants to avoid leaks, it should register a 
 single callback which will then walk the current live resources and protect 
 them.

I guess that the custom usage makes sense.
I'd just like a note in the doc, if possible ;-)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Aaron Iles]

Changes by Aaron Iles aaron.i...@gmail.com:


--
nosy: +aliles

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Dwayne Litzenberger]

Changes by Dwayne Litzenberger dl...@dlitz.net:


--
nosy: +DLitz

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
nosy: +neologix

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[STINNER Victor]

STINNER Victor added the comment:

 Might make sense to put this in atexit.atfork() to avoid small-module 
 inflation?

It sounds strange to mix at exit and at fork in the same module.
Both are very different.

2013/1/13 Arfrever Frehtes Taifersar Arahesis rep...@bugs.python.org:

 Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com:


 --
 nosy: +Arfrever

 ___
 Python tracker rep...@bugs.python.org
 http://bugs.python.org/issue16500
 ___

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Marc-Andre Lemburg]

Marc-Andre Lemburg added the comment:

On 13.01.2013 00:37, STINNER Victor wrote:
 By the way, OpenSSL expects that its PRNG is reseed somehow (call RNG_add) 
 after a fork. I wrote a patch for OpenSSL, but I don't remember if I sent it 
 to OpenSSL.
 https://bitbucket.org/haypo/hasard/src/4a1be69a47eb1b2ec7ca95a341d4ca953a77f8c6/patches/openssl_rand_fork.patch?at=default

Apparently not, and according to this thread, they don't think
this is an OpenSSL problem to solve:

http://openssl.6102.n7.nabble.com/recycled-pids-causes-PRNG-to-repeat-td41669.html

Note that you don't have to reseed the RNG just make sure that the
two forks use different sequences. Simply adding some extra data
in each process will suffice, e.g. by adding the PID of the new process
to the RNG pool. This is certainly doable without any major CPU
overhead :-)

--
nosy: +lemburg

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Antoine Pitrou]

Antoine Pitrou added the comment:

 It sounds strange to mix at exit and at fork in the same module.
 Both are very different.

That's true. The sys module would probably be the right place for both 
functionalities.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Georg Brandl]

Georg Brandl added the comment:

Might make sense to put this in atexit.atfork() to avoid small-module inflation?

--
nosy: +georg.brandl

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[Arfrever Frehtes Taifersar Arahesis]

Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com:


--
nosy: +Arfrever

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16500] Add an 'atfork' module

[STINNER Victor]

STINNER Victor added the comment:

The tempfile module has a specialized RNG that re-initialized the RNG after 
fork() by comparing os.getpid() to an instance variable every time the RNG is 
accessed. The check can be replaced with an afterfork callback.

By the way, OpenSSL expects that its PRNG is reseed somehow (call RNG_add) 
after a fork. I wrote a patch for OpenSSL, but I don't remember if I sent it to 
OpenSSL.
https://bitbucket.org/haypo/hasard/src/4a1be69a47eb1b2ec7ca95a341d4ca953a77f8c6/patches/openssl_rand_fork.patch?at=default

Reseeding tempfile PRNG is useless (but spend CPU/memory/hang until we have 
enough entropy?) if the tempfile is not used after fork. I like the current 
approach.

--

I'm not saying that a new atfork module would not help, just that the specific 
case of tempfile should be discussed :-) I like the idea of a generic module to 
call code after fork.

--
nosy: +haypo
title: Add an 'afterfork' module - Add an 'atfork' module

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16500
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com