2013/7/28 Antoine Pitrou :

...

(C) Should we handle standard streams (0: stdin, 1: stdout, 2: stderr) differently? For example, os.dup2(fd, 0) should make the file descriptor 0 (stdin) inheritable or non-inheritable? On Windows, os.set_inheritable(fd, False) fails (error 87, invalid argument) on standard streams (0, 1, 2) and copies of the standard streams (created by os.dup()).

I have been advocating for that, but I now realize that special-casing these three descriptors in a myriad of fd-creating functions isn't very attractive. (if a standard stream fd has been closed, any fd-creating function can re-create that fd: including socket.socket(), etc.)

So perhaps only the *original* standard streams should be left inheritable?

Having stdin/stdout/stderr cloexec (e.g. after a dup() to redirect to a log file, a socket...) will likely break a lot of code, e.g. code using os.system(), or code calling exec manually (and I'm sure there's a bunch of it). Also, it'll be puzzling to have syscalls automatically set the cloexec flag. I guess a lot of people doing system programming with Python will get bitten, but that's a discussion we already had months ago... cf

...

Having stdin/stdout/stderr cloexec (e.g. after a dup() to redirect to a log file, a socket...) will likely break a lot of code, e.g. code using os.system(), or code calling exec manually (and I'm sure there's a bunch of it).

Hmm. os.exec*() could easily make standard streams non-CLOEXEC before calling the underlying C library function. Things are more annoying for os.system(), though.

...

Also, it'll be puzzling to have syscalls automatically set the cloexec flag. I guess a lot of people doing system programming with Python will get bitten, but that's a discussion we already had months ago...

Perhaps this advocates for a global flag, e.g. sys.set_default_fd_inheritance(), with False (non-inheritable) being the default for sanity and security.

This looks more and more like PEP 433 :-) And honestly, when I think about it, I think that this whole mess is a solution looking for a problem. If we don't want to inherit file descriptors in child processes, the answer is simple: the subprocess module (this fact is not even mentioned in the PEP). If a user wants to use the execve() syscall directly, then he should be aware of the implications. I don't think that patching half the stdlib and complicating the API of many functions is the right way to do this. The stdlib should be updated to replace the handful of places where exec() is called explicitly by subprocess (the only one I can think on top of my head is http.server.CGIHTTPRequestHandler (issue #16945)), otherwise that's about it. cf

Regards

Antoine.

_______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/cf.natali%40gmail.com

2013/7/30 Antoine Pitrou :

Le Tue, 30 Jul 2013 09:09:38 +0200, Charles-François Natali a écrit :

...

This looks more and more like PEP 433 :-)

And honestly, when I think about it, I think that this whole mess is a solution looking for a problem. If we don't want to inherit file descriptors in child processes, the answer is simple: the subprocess module (this fact is not even mentioned in the PEP).

This is a good point. Are there any reasons (other than fd inheritance) not to use subprocess? If there are, perhaps we should try to eliminate them by improving subprocess.

On Windows, inheritable handles (including open files) are still inherited when a standard stream is overriden in the subprocess module (default value of close_fds is set to False in this case). This issue cannot be solved (at least, I don't see how): it is a limitation of Windows. bInheritedHandles must be set to FALSE (inherit *all* inheritable handles) when handles of standard streams are specified in the startup information of CreateProcess(). Victor

On 02/08/2013 7:44am, Charles-François Natali wrote:

Then how about changing the default to creating file descriptors unheritable on Windows (which is apparently the default)? Then you can implement keep_fds by setting them inheritable right before creation, and resetting them right after: sure there's a race in a multi-threaded program, but AFAICT that's already the case right now, and Windows API doesn't leave us any other choice. Amusingly, they address this case by recommending putting process creation in a critical section: http://support.microsoft.com/kb/315939/en-us

This way, we keep default platform behavior on Unix and on Windows (so user using low-level syscalls/APIs won't be surprised), and we have a clean way to selectively inherit FD in child processes through subprocess.

http://bugs.python.org/issue16500 is a proposal/patch for adding atfork. But it also has a public recursive lock which is held when starting processes using fork()/subprocess/multiprocessing. This is so that users can safely manipulate fds while holding the lock, without these sorts of race conditions. For example with atfork.getlock(): fd = os.open("somefile", os.O_CREAT | os.O_WRONLY, 0600) flags = fcntl.fcntl(fd, fcntl.F_GETFL) fcntl.fcntl(fd, fcntl.F_SETFL, flags | fcntl.FD_CLOEXEC) -- Richard

On 02/08/2013 12:30pm, Victor Stinner wrote:

Is it possible to implement atfork on Windows?

On Windows the patch does expose atfork.getlock() and uses it in subprocess. (It should also modify os.spawn?(), os.startfile() etc.) But atfork.atfork() is Unix only.

A Python lock would be ignored by other C threads. It is unsafe if Python is embedded.

True. -- Richard

http://support.microsoft.com/kb/315939/en-us Ah yes, we may implement pass_handles on Windows using a critical section to inherit *handles*. File descriptors cannot be inherited using CreateProcess(), only using spawn(). Or can we rely on the undocumented fields used by spawn()? Victor

On 30/07/2013 8:09am, Charles-François Natali wrote:

If we don't want to inherit file descriptors in child processes, the answer is simple: the subprocess module (this fact is not even mentioned in the PEP).

Note that on Windows subprocess has no equivalent of a passfds argument, and if you redirect the standard streams then you are forced to inherit all inheritable handles. -- Richard

On 30/07/2013 11:52am, Victor Stinner wrote:

You can redirect standard streams (stdin, stdout, stderr) using the startup info structure:

startupinfo.dwFlags |= _winapi.STARTF_USESTDHANDLES startupinfo.hStdInput = p2cread startupinfo.hStdOutput = c2pwrite startupinfo.hStdError = errwrite

The documentation for STARTUPINFO says this about STARTF_USESTDHANDLES: If this flag is specified when calling one of the process creation functions, the handles must be inheritable and the function's bInheritHandles parameter must be set to TRUE. So, as I said, if you redirect the streams then you inherit all inheritable handles. -- Richard

On 02/08/2013 12:59am, Victor Stinner wrote:

On Windows, a file cannot be removed if at least one process opened it. If you create a temporary file, run a program, and delete the temporary file: the deletion fails if the program inherited the file and the program is not done before the deletion. Is this correct?

It depends whether the handles use FILE_SHARE_DELETE. The Python module tempfile uses the O_TEMPORARY flag which implies FILE_SHARE_DELETE. This means that the file *can* be deleted/moved while there are open handles. Annoyingly the folder containing that file still cannot be deleted while there are open handles. -- Richard

On 02/08/2013 1:21am, Victor Stinner wrote:

2013/7/30 Victor Stinner :

...

I would be nice to have a "pass_handles" on Windows.

I'm not sure that it's possible to implement this atomically. It's probably better to leave the application to choose how the inheritance is defined.

Example:

for handle in handles: os.set_inheritable(handle, True) subprocess.call(...) for handle in handles: os.set_inheritable(handle, False)

This example is safe if the application has a single thread (if a single thread spawn new programs). Making handles non-inheritable again may be useless.

If we have a recursive lock which is always held when Python starts a process then you could write: with subprocess.getlock(): for handle in handles: os.set_inheritable(handle, True) subprocess.call(...) for handle in handles: os.set_inheritable(handle, False) This should be used by fork() and multiprocessing too. -- Richard

On Mon, 29 Jul 2013 23:42:36 +0200 Victor Stinner wrote:

...

...

So perhaps only the *original* standard streams should be left inheritable?

I plan to only change functions *creating* (and replacing) new file descriptors. Existing file descriptors (like 0, 1, 2) are unchanged.

You can create fds 0, 1 and 2 if they were closed before.

...

Having stdin/stdout/stderr cloexec (e.g. after a dup() to redirect to a log file, a socket...) will likely break a lot of code, e.g. code using os.system(), or code calling exec manually (and I'm sure there's a bunch of it).

I propose to add just one special case os.dup2(fd, fd2): if fd2 < 3, fd2 is set to inheritable (otherwise, fd2 is set to non-inheritable). os.dup2() is commonly used to replace stdin, stdout and stderr between fork and exec. The http.server (cgi server) and pty modules use dup2() for example

Mmh. Doing it only in dup2() sounds like too much special casing for me. I would prefer adding a new explicit parameter: os.dup2(fd, fd2, inheritable=False) (as a bonus, it can be implemented using dup3() under (GNU/)Linux) Regards Antoine.

2013/7/28 Antoine Pitrou :

...

(B) Should subprocess make the file descriptors of pass_fds inheritable? If yes, should it be done before or after the fork? If it is done after the fork and before exec, it only affects the child process, at least on Linux (the file descriptor is still non-inheritable in the parent process).

If it is done, it should definitely be done after the fork, IMO. subprocess shouldn't have any long-lasting effects on the process.

I modified the subprocess module to make fds of pass_fds inheritable. http://hg.python.org/features/pep-446/rev/75e5d34898aa If we don't do that, it will probably break all applications using pass_fds (and so the backward compatibility). Victor

2013/8/2 Victor Stinner :

2013/7/28 Antoine Pitrou :

...

...

(A) How should we support support where os.set_inheritable() is not supported? Can we announce that os.set_inheritable() is always available or not? Does such platform exist?

FD_CLOEXEC is POSIX: http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html

Ok, but this information does not help me. Does Python support non-POSIX platforms? (Windows has HANDLE_FLAG_INHERIT.)

If we cannot answer to my question, it's safer to leave os.get/set_inheritable() optional (need hasattr in tests for example).

On Unix platforms, you should always have FD_CLOEXEC. If there were such a platform without FD inheritance support, then it would probably make sense to make it a no-op anyway. cf

Le Fri, 2 Aug 2013 02:27:43 +0200, Victor Stinner a écrit :

2013/7/28 Antoine Pitrou :

...

...

(A) How should we support support where os.set_inheritable() is not supported? Can we announce that os.set_inheritable() is always available or not? Does such platform exist?

FD_CLOEXEC is POSIX: http://pubs.opengroup.org/onlinepubs/9699919799/functions/fcntl.html

Ok, but this information does not help me. Does Python support non-POSIX platforms? (Windows has HANDLE_FLAG_INHERIT.)

Python works under POSIX and Windows. Not sure what else you are thinking about :-) Regards Antoine.