Alexey Izbyshev <izbys...@ispras.ru> added the comment:

> It should be compared to the current code. Currently, _posixsubprocess uses a 
> loop calling execv(). I don't think that calling posix_spawn() in a loop 
> until one doesn't fail is more inefficient.

> The worst case would be when applying process attributes and run file actions 
> would dominate performances... I don't think that such case exists. Syscalls 
> like dup2() and close() should be way faster than the final successful 
> execv() in the overall posix_spawn() call. I'm talking about the case when 
> the program exists.

I had vfork() used internally by posix_spawn() in mind when I considered 
repeatedly calling it "prohibitively expensive". While vfork() is much cheaper 
than fork(), it doesn't mean that its performance is comparable to dup2() and 
close(). But on systems where posix_spawn is a syscall the overhead could 
indeed be lesser. Anyway, it should be measured.

>> Iterate over PATH entries and perform a quick check for common exec errors 
>> (ENOENT, ENOTDIR, EACCES) manually (e.g. by stat()).

> I dislike this option. There is a high risk of race condition if the program 
> is created, deleted or modified between the check and exec. It can cause 
> subtle bugs, or worse: security vulnerabilities. IMHO the only valid check, 
> to test if a program exists, is to call exec().

While exec() is of course cleaner, IMHO races don't matter in this case. If our 
stat()-like check fails, we could as well imagine that it is exec() that failed 
(while doing the same checks as our stat()), so it doesn't matter what happens 
with the tested entry afterwards. If the check succeeds, we simply call 
posix_spawn() that will perform the same checks again. If any race condition 
occurs, the problem will be detected by posix_spawn(). 

> Alexey: Do you want to work on a PR to reimplement the "executable_list" and 
> loop used by subprocess with _posixsubproces?

I'm currently focused on researching whether it's possible to use vfork()-like 
approach instead of fork() on Linux, so if anyone wants to implement the PR you 
suggested, feel free to do it.

----------

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

Reply via email to