STINNER Victor <victor.stin...@haypocalc.com> added the comment:

fd_status.py:

+try:
+    _MAXFD = os.sysconf("SC_OPEN_MAX")
+except:
+    _MAXFD = 256

It looks like this code (256 constant) comes from subprocess.py. Is that a good 
value? On Linux, SC_OPEN_MAX is usually 1024, and it can be 4096. Should we 
keep the default value 256, or use 1024 or 4096 instead? I don't know on which 
OS SC_OPEN_MAX is missing.

--

fd_status.py: isopen(fd) uses fcntl.fcntl(fd, fcntl.F_GETFD, 0). Is it always 
available? If not, we can add fstat() as a fallback. The buildbots will tell us 
:-)

--

subprocess.py: _create_pipe() doesn't use pipe2() because Python doesn't 
provide pipe2(). We should maybe add it to the posix module (open maybe a new 
issue for that).

> The CLOEXEC flag needs to be set atomically (or at least in a way
> that another subprocess won't start in the middle of it)

For the Python implementation, the GIL is not enough to ensure the atomicity of 
a process creation. That's why _posixsubprocess was created. I suppose that 
other parts of subprocess are not atomic and a lock is required to ensure that 
the creation of subprocess is atomic.

--

_posixsubprocess.c: is FD_CLOEXEC flag always available? fcntlmodule.c uses a 
"#ifdef FD_CLOEXEC".

Can you add a comment to explain why you can release the GIL here? (because the 
operation is atomic)

+    Py_BEGIN_ALLOW_THREADS
+    res = pipe2(fds, O_CLOEXEC);
+    Py_END_ALLOW_THREADS

--

test_subprocess.py: test_pipe_cloexec() and test_pipe_cloexec_real_tools() 
should maybe be skipped if fcntl has no attribute FD_CLOEXEC.

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue7213>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to