5 ms·

Subprocess: Don't close all file descriptors by default (close_fds=False)

35 points by luu 2y ago

williamjackson 2y ago
https://github.com/python/cpython/issues/86904 https://github.com/python/cpython/issues/86904
- loeg 2y ago
  (The comments before 2022 are all the same; the newest comment is only on Github.)
oseityphelysiol 2y ago
Of all the quirks with process spawning in posix keeping file descriptors open is the most problematic one I encountered. This bit into my ass while implementing a C library to have proper process spawning and stdio handling in LUA. I really wish file descriptors were non inheritable by default.
- teddyh 2y ago
  > I really wish file descriptors were non inheritable by default. In Python 3.4, they are (released ten years ago).
  - int_19h 2y ago
    But in POSIX, they are not, so any module implemented in C is still potentially problematic.
    - teddyh 2y ago
      Only if that C-implemented module uses raw C to create file descriptors. And if the module has not gotten an update in the past ten years to fix the problem.
    - jcalvinowens 2y ago
      This can help: https://man7.org/linux/man-pages/man2/close_range.2.html https://man7.org/linux/man-pages/man2/close_range.2.html close_range(3, ~0U, CLOSE_RANGE_UNSHARE); execve(....); ...but it's not very portable yet.
      - deleted 2y ago
        [deleted]
      - oseityphelysiol 2y ago
        I’ve never heard of it before. I work in an embedded environment where kernel version is known beforehand, so portability won’t be an issue. Thanks.
htfy96 2y ago
Meanwhile VSCode's terminal still leaks Electron fds: https://github.com/microsoft/node-pty/issues/657 https://github.com/microsoft/node-pty/issues/657 I get this proposal's rationale, but it seems that it would implicitly make fd leaks more prone in python programs
loeg 2y ago
(2020), or perhaps 2021.
teddyh 2y ago
The counter-arguments presented seem persuasive. This was originally submitted in 2020, and closed in 2022. Why is it relevant or interesting today?
- deleted 2y ago
  [deleted]
mmastrac 2y ago
FDs really should have been opt-in for inheritability from the start, with the possible exception of stdio. Inheritability being an fcntl is definitely one of the worst bits -- if the APIs for fork()/etc were designed today it would probably take a list of FDs that would be dup2'd in the new child process.
- stefan_ 2y ago
  FDs optin, memory too given the insane performance pitfalls architectural issues and pernicious security problems (think cloning a CSPRNGs internal state) and suddenly you realize CreateProcessA was and always has been the superior API.
  - adzm 2y ago
    > suddenly you realize CreateProcessA was and always has been the superior API Throw in IOCP and you realize that NT is a pretty solid, well-thought-out OS
    - hyperman1 2y ago
      Speaking as a linux addict from the 90s: The NT kernel was great. The windows GDI and USER were reasonable but a bit warty, and not fully up to date for current graphics works. COM as interoperability protocol was quite good. The windows UI from 95 to win2k was also great. Really apart from the lack of unix shell like automation and some serious security holes, this was a good system. Unfortunately, starting with XP, the end user was not Microsoft's center of attention anymore, and the OS gets worse every new version since.
- loeg 2y ago
  > FDs really should have been opt-in for inheritability from the start, with the possible exception of stdio. Yes. > Inheritability being an fcntl is definitely one of the worst bits Well, or you can use O_CLOEXEC in most APIs that create an fd.
  - 2y ago
nasretdinov 2y ago
Go goes slightly further and opens all descriptors with O_CLOEXEC by default, so if you ever execute an external command you have to go out of your way to preserve any descriptors, which is really nice in my opinion
- loeg 2y ago
  Python has done the same thing for the past 10 years for fds created by the runtime (Python 3.4). But 3rd party extensions / modules may create non-CLOEXEC fds.
Pesthuf 2y ago
> On macOS, posix_spawn() is even a syscall. First time I hear about this, interesting. I wonder what the performance benefits are like.
zitterbewegung 2y ago
Dang: Link should be https://github.com/python/cpython/issues/86904 https://github.com/python/cpython/issues/86904

35 points by luu 2y ago

williamjackson 2y ago

https://github.com/python/cpython/issues/86904 https://github.com/python/cpython/issues/86904

loeg 2y ago
(The comments before 2022 are all the same; the newest comment is only on Github.)

oseityphelysiol 2y ago

Of all the quirks with process spawning in posix keeping file descriptors open is the most problematic one I encountered. This bit into my ass while implementing a C library to have proper process spawning and stdio handling in LUA. I really wish file descriptors were non inheritable by default.

teddyh 2y ago
> I really wish file descriptors were non inheritable by default. In Python 3.4, they are (released ten years ago).
- int_19h 2y ago
  But in POSIX, they are not, so any module implemented in C is still potentially problematic.
  - teddyh 2y ago
    Only if that C-implemented module uses raw C to create file descriptors. And if the module has not gotten an update in the past ten years to fix the problem.
  - jcalvinowens 2y ago
    This can help: https://man7.org/linux/man-pages/man2/close_range.2.html https://man7.org/linux/man-pages/man2/close_range.2.html close_range(3, ~0U, CLOSE_RANGE_UNSHARE); execve(....); ...but it's not very portable yet.
    - deleted 2y ago
      [deleted]
    - oseityphelysiol 2y ago
      I’ve never heard of it before. I work in an embedded environment where kernel version is known beforehand, so portability won’t be an issue. Thanks.

htfy96 2y ago

Meanwhile VSCode's terminal still leaks Electron fds: https://github.com/microsoft/node-pty/issues/657 https://github.com/microsoft/node-pty/issues/657 I get this proposal's rationale, but it seems that it would implicitly make fd leaks more prone in python programs

loeg 2y ago

(2020), or perhaps 2021.

teddyh 2y ago

The counter-arguments presented seem persuasive. This was originally submitted in 2020, and closed in 2022. Why is it relevant or interesting today?

deleted 2y ago
[deleted]

mmastrac 2y ago

FDs really should have been opt-in for inheritability from the start, with the possible exception of stdio. Inheritability being an fcntl is definitely one of the worst bits -- if the APIs for fork()/etc were designed today it would probably take a list of FDs that would be dup2'd in the new child process.

stefan_ 2y ago
FDs optin, memory too given the insane performance pitfalls architectural issues and pernicious security problems (think cloning a CSPRNGs internal state) and suddenly you realize CreateProcessA was and always has been the superior API.
- adzm 2y ago
  > suddenly you realize CreateProcessA was and always has been the superior API Throw in IOCP and you realize that NT is a pretty solid, well-thought-out OS
  - hyperman1 2y ago
    Speaking as a linux addict from the 90s: The NT kernel was great. The windows GDI and USER were reasonable but a bit warty, and not fully up to date for current graphics works. COM as interoperability protocol was quite good. The windows UI from 95 to win2k was also great. Really apart from the lack of unix shell like automation and some serious security holes, this was a good system. Unfortunately, starting with XP, the end user was not Microsoft's center of attention anymore, and the OS gets worse every new version since.
loeg 2y ago
> FDs really should have been opt-in for inheritability from the start, with the possible exception of stdio. Yes. > Inheritability being an fcntl is definitely one of the worst bits Well, or you can use O_CLOEXEC in most APIs that create an fd.
- 2y ago

nasretdinov 2y ago

Go goes slightly further and opens all descriptors with O_CLOEXEC by default, so if you ever execute an external command you have to go out of your way to preserve any descriptors, which is really nice in my opinion

loeg 2y ago
Python has done the same thing for the past 10 years for fds created by the runtime (Python 3.4). But 3rd party extensions / modules may create non-CLOEXEC fds.

Pesthuf 2y ago

> On macOS, posix_spawn() is even a syscall. First time I hear about this, interesting. I wonder what the performance benefits are like.

zitterbewegung 2y ago

Dang: Link should be https://github.com/python/cpython/issues/86904 https://github.com/python/cpython/issues/86904