STINNER Victor added the comment:
I close the issue as "not a bug". If you disagree, please comment the issue :-)
Steven Stewart-Gallus:
> In the code FD_DIR is "/proc/self/fd" on Linux. I'm not sure this code
> is correct. This seems as if it would have the same problems as
> iterating over
STINNER Victor added the comment:
Dan Snider: "On Android, if os.close_range closes the file descriptor of the
scandir iterator, the interpreter immediately crashes"
I don't see anything wrong with Python. It's not different than calling
os.close(3).
If it hurts, don't do that :-)
Dan Snider added the comment:
On Android, if os.close_range closes the file descriptor of the scandir
iterator, the interpreter immediately crashes eg:
>>> import os
>>> os.scandir()
>>> os.closerange(3,)
fdsan: attempted to close file descriptor 3, expected to be unowned, actually
STINNER Victor added the comment:
Linux 5.9 added close_range() syscall that we can use.
Linux 5.10 may get a new CLOSE_RANGE_CLOEXEC flag for close_range().
https://lwn.net/Articles/837816/
We may use close_range() rather than iterating on /proc/self/fd/ in user space.
It may be faster and
Gregory P. Smith added the comment:
The Linux kernel code is not sufficiently easy to follow to understand if it
has this issue or if it pre-creates the dirent structures for all fds at
opendir time for /proc/self/fd or if it is iterating through the list of fds in
sorted order so an older
Serhiy Storchaka added the comment:
It is Modules/_posixsubprocess.c.
We still do not have a reproducer, so I am not sure that the code has this
problem.
--
nosy: +serhiy.storchaka
status: pending -> open
___
Python tracker
Irit Katriel added the comment:
I can't find _posixsubmodules.c or _close_open_fd_range_safe in the codebase.
Is this issue out of date?
--
nosy: +iritkatriel
resolution: -> out of date
status: open -> pending
___
Python tracker
Steven Stewart-Gallus added the comment:
Okay, I made a patch that I hoped dealt with all the criticisms and that fixed
up a problem I noted myself.
--
Added file: http://bugs.python.org/file35599/fixed-setcloexec.patch
___
Python tracker
Changes by Steven Stewart-Gallus sstewartgallu...@mylangara.bc.ca:
Removed file: http://bugs.python.org/file35599/fixed-setcloexec.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21627
___
Changes by Steven Stewart-Gallus sstewartgallu...@mylangara.bc.ca:
Added file: http://bugs.python.org/file35600/fixed-setcloexec.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21627
___
STINNER Victor added the comment:
The _posixsubprocess module is not compiled on Windows.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21627
___
Steven Stewart-Gallus added the comment:
Okay, now I'm confused. How would I conditionally compile and use the
setcloexec object and header on POSIX platforms and not on Windows?
--
___
Python tracker rep...@bugs.python.org
Steven Stewart-Gallus added the comment:
Okay, so the Python directory seems to be where wrappers over low level or
missing functionality is placed. So I guess I should make a _Py_setcloexec
function in a Python/setcloexec.c and a definition in Include/setcloexec.h?
--
Changes by Berker Peksag berker.pek...@gmail.com:
--
nosy: +gregory.p.smith
versions: +Python 3.5
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21627
___
Changes by STINNER Victor victor.stin...@gmail.com:
--
nosy: +neologix, pitrou
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21627
___
___
STINNER Victor added the comment:
I have no opinon on close() vs setting CLOEXEC flag, but you should use
_Py_set_inheritable(fd, 0, NULL). _Py_set_inheritable() is able to set the
CLOEXEC flag in a single syscall, instead of 2 syscalls.
_close_fds_by_brute_force() is already very slow when
STINNER Victor added the comment:
Since Python 3.4, all file descriptors directly created by Python are marked as
non-inheritable. It should now be safe to use subprocess with close_fds=False,
but the default value was not changed because third party modules (especially
code written in C) may
Steven Stewart-Gallus added the comment:
It occurred to me that the current patch I have is wrong and that using
_Py_set_inheritable is wrong because EBADF can occur in the brute force version
which in the case of _Py_set_inheritable raises an error which I am not sure is
asynch signal safe.
New submission from Steven Stewart-Gallus:
Hello,
I noticed some possible bad behaviour while working on Python issue
21618 (see http://bugs.python.org/issue21618). Python has the
following code in _posixsubmodules.c for closing extra files before
spawning a process:
static void
Steven Stewart-Gallus added the comment:
Okay, I've made a simple proof of concept patch.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21627
___
Steven Stewart-Gallus added the comment:
Gah, I had trouble figuring out mecurial.
Close files after reading open files and not concurrently
This commit removes the old behaviour of closing files concurrently
with reading the system's list of open files and instead closes the
files after the
21 matches
Mail list logo
