[issue35823] Use vfork() in subprocess on Linux

Марк Коренберг added the comment: Yes, you are almost right. Error-path is not so clear (it is discussed in another issue), but in general, yes, my previous comment is wrong. So, finally, there are no bugs around the stack at all. -- ___ Python

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: The preceding comment is wrong, see discussion in #47245 and https://bugzilla.kernel.org/show_bug.cgi?id=215813#c14 for explanation of why that bug report is irrelevant for CPython. -- ___ Python tracker

[issue35823] Use vfork() in subprocess on Linux

Марк Коренберг added the comment: See #47245. https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/spawni.c#L309 In short - do not use vfork(). Use clone(CLONE_VM | CLONE_VFORK). And build separate stack. Current implementation is heavily broken. Another guy has failed:

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: Performance improvement measured on a 1.4Ghz quad aarch64 A57 (nvidia jetson nano): #define VFORK_USABLE 1 test_subprocess: 36.5 seconds #undef VFORK_USABLE test_subprocess: 45 seconds Nice. I really didn't expect a 20% speedup on its testsuite alone!

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: Nice links. LOL, yes, musl source was going to be my next stop if the larger libc sources proved impossible for a mere mortal to reason about. :) regarding macOS, agreed. If someone needs vfork() to work there, I believe it could be made to. Options

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: @ronaldoussoren > I'd prefer to not use vfork on macOS. For one I don't particularly trust that > vfork would work reliably when using higher level APIs, but more importantly > posix_spawn on macOS has some options that are hard to achieve otherwise and >

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: > regarding excluding the setsid() case: I was being conservative as I couldn't > find a reference of what was and wasn't allowed after vfork. Yes, there is no list of functions allowed after vfork(), except for the conservative POSIX.1 list consisting

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: New changeset be3c3a0e468237430ad7d19a33c60d306199a7f2 by Gregory P. Smith in branch 'master': bpo-35823: Allow setsid() after vfork() on Linux. (GH-22945) https://github.com/python/cpython/commit/be3c3a0e468237430ad7d19a33c60d306199a7f2 --

[issue35823] Use vfork() in subprocess on Linux

Change by Gregory P. Smith : -- pull_requests: +21863 pull_request: https://github.com/python/cpython/pull/22945 ___ Python tracker ___

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: regarding excluding the setsid() case: I was being conservative as I couldn't find a reference of what was and wasn't allowed after vfork. I found one thing suggesting that on macOS setsid() was not safe after vfork(). But that appeared to be a

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: New changeset 473db47747bb8bc986d88ad81799bcbd88153ac5 by Alexey Izbyshev in branch 'master': bpo-35823: subprocess: Fix handling of pthread_sigmask() errors (GH-22944) https://github.com/python/cpython/commit/473db47747bb8bc986d88ad81799bcbd88153ac5

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: > Thank you for taking this on! I'm calling it fixed for now as the buildbots > are looking happy with it. If issues with it arise we can address them. Thank you for reviewing and merging! Using POSIX_CALL for pthread_sigmask() is incorrect, however,

[issue35823] Use vfork() in subprocess on Linux

Change by Alexey Izbyshev : -- pull_requests: +21862 pull_request: https://github.com/python/cpython/pull/22944 ___ Python tracker ___

[issue35823] Use vfork() in subprocess on Linux

Ronald Oussoren added the comment: > From what I can tell, vfork probably also works on macOS (darwin). > > Lets let this run for a bit on Linux and it can be a separate issue to > open vfork usage up to other platforms. I'd prefer to not use vfork on macOS. For one I don't particularly

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: Thank you for taking this on! I'm calling it fixed for now as the buildbots are looking happy with it. If issues with it arise we can address them. -- resolution: -> fixed stage: commit review -> resolved status: open -> closed

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: > * To avoid repeating long parameter lists in several functions, we can move > them to a struct. The downside is that we'd need to convert child_exec() to > use that struct all over the place. I don't have a strong preference here. Agreed that the long

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: now waiting to see how happy all of the buildbots are... We currently have a `__linux__` check in the code deciding VFORK_USABLE. >From what I can tell, vfork probably also works on macOS (darwin). Lets let this run for a bit on Linux and it can be a

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: New changeset 976da903a746a5455998e9ca45fbc4d3ad3479d8 by Alexey Izbyshev in branch 'master': bpo-35823: subprocess: Use vfork() instead of fork() on Linux when safe (GH-11671)

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: I've updated my PR. * After a discussion with Alexander Monakov (a GCC developer), moved vfork() into a small function to isolate it from both subprocess_fork_exec() and child_exec(). This appears to be the best strategy to avoid -Wclobbered false

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: Well, much later than promised, but I'm picking it up. Since in the meantime support for setting uid/gid/groups was merged, and I'm aware about potential issues with calling corresponding C library functions in a vfork()-child, I asked a question on musl

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: No objections, it would be great to see this finished up and land. I've encountered a minority of users who are using a wrapped vfork-based C/C++ library for process spawning as fork+exec isn't fast enough for them. --

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: I'd really like to get this merged eventually because vfork()-based solution is fundamentally more generic than posix_spawn(). Apart from having no issue with close_fds=True, it will also continue to allow subprocess to add any process context tweaks

[issue35823] Use vfork() in subprocess on Linux

Yonatan Goldschmidt added the comment: With issue35537 merged, do we have any intention to get on with this? From what I can tell, it provides roughly the same performance benefit - but works also with close_fds=True, which is the default of Popen, so IMO it's a worthy addition. I tested a

[issue35823] Use vfork() in subprocess on Linux

Change by Yonatan Goldschmidt : -- nosy: +Yonatan Goldschmidt ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35823] Use vfork() in subprocess on Linux

Change by STINNER Victor : -- nosy: -vstinner ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35823] Use vfork() in subprocess on Linux

Change by Gregory P. Smith : -- versions: +Python 3.10 -Python 3.8 ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: I've been struggling with fixing spurious -Wclobbered GCC warnings. Originally, I've got the following: /scratch2/izbyshev/cpython/Modules/_posixsubprocess.c: In function ‘subprocess_fork_exec’: /scratch2/izbyshev/cpython/Modules/_posixsubprocess.c:612:15:

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: Thank you for the review and your thoughts, Gregory. > With this in place we may want to make the _use_posix_spawn() logic in > subprocess.py stricter? That could be its own followup PR. Yes, I think we can always use vfork() on Linux unless we find some

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: > * Decide whether "setxid problem"[5] is important enough to worry about. > [5] https://ewontfix.com/7 This is a scary issue. But I think a reasonable approach could be to never use vfork when running as whatever we choose to define a "privileged user"

[issue35823] Use vfork() in subprocess on Linux

Gregory P. Smith added the comment: Thanks for your _extremely detailed_ analysii of the (often sad) state of posix_spawn() on platforms in the world today. My first reaction to this was "but then we'll be owning our own custom posix_spawn-like implementation as if we'll do better at it

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: I've checked subprocess.Popen() error reporting in QEMU user-mode and WSL and confirm that it works both with my patch (vfork/exec) and the traditional fork/exec, but doesn't work with glibc's posix_spawn. The first command below uses posix_spawn()

[issue35823] Use vfork() in subprocess on Linux

Ronald Oussoren added the comment: BTW. I hadn't noticed _close_open_fds_safe, that should be safe when using vfork(). Finally: I have no strong opinion on this patch, your explanation looks fine and I'm not up-to-speed w.r.t. implementation details of vfork and the like to feel

[issue35823] Use vfork() in subprocess on Linux

Change by STINNER Victor : -- pull_requests: +11492, 11493 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35823] Use vfork() in subprocess on Linux

Change by STINNER Victor : -- pull_requests: +11492 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35823] Use vfork() in subprocess on Linux

Alexey Izbyshev added the comment: > W.r.t. closing all file descriptors > 2: posix_spawn_file_actions_addclose > can do this when using posix_spawn. That would have a performance cost, you'd > basically have to resort to closing all possible file descriptors and cannot > use the smarter

[issue35823] Use vfork() in subprocess on Linux

Change by Kubilay Kocak : -- nosy: +koobs ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue35823] Use vfork() in subprocess on Linux

Ronald Oussoren added the comment: Issue #34663 contains some earlier discussion about vfork, in the context of supporting a specific posix_spawn flag on Linux. W.r.t. closing all file descriptors > 2: posix_spawn_file_actions_addclose can do this when using posix_spawn. That would have a

[issue35823] Use vfork() in subprocess on Linux

Change by Alexey Izbyshev : -- keywords: +patch, patch pull_requests: +11484, 11485 stage: -> patch review ___ Python tracker ___

[issue35823] Use vfork() in subprocess on Linux

Change by Alexey Izbyshev : -- keywords: +patch pull_requests: +11484 stage: -> patch review ___ Python tracker ___ ___

[issue35823] Use vfork() in subprocess on Linux

Change by Alexey Izbyshev : -- keywords: +patch, patch, patch pull_requests: +11484, 11485, 11486 stage: -> patch review ___ Python tracker ___

[issue35823] Use vfork() in subprocess on Linux

New submission from Alexey Izbyshev : This issue is to propose a (complementary) alternative to the usage of posix_spawn() in subprocess (see bpo-35537). As mentioned by Victor Stinner in msg332236, posix_spawn() has the potential of being faster and safer than fork()/exec() approach.