Re: system and popen fail in case of big application

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 12/09/2018 13:29, Zack Weinberg wrote:
> On Tue, Sep 11, 2018 at 4:16 PM >
> Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>> On 09/09/2018 19:44, Zack Weinberg wrote:
>>> On Sun, Sep 9, 2018 at 6:16 PM Sergey Melnikov
>>> <melnikov.sergey.v@gmail.com> wrote:
>>>> So, my question is if does it worth to rewrite existing popen/system
>>>> calls with posix_spawn as StackOverflow recommends? If so, I may
>>>> implement and contribute this to glibc.
>>>
>>> What would be worthwhile is to change popen and system so that they
>>> internally use *vfork*, on systems where vfork is not the same as
>>> fork. posix_spawn is just an elaborate and inconvenient wrapper around
>>> vfork, and I don't think we should be using it internally or
>>> encouraging its use by others.  Also, unlike vfork, it does not
>>> guarantee to avoid the problem you are experiencing.
>>
>> We changed posix_spawn to exactly avoid call vfork due its inherent usage
>> issue with signal handlers (BZ#14750), cancellation state being shared
>> with the parent (BZ#10354), and the vfork limitation of possible parent
>> clobber due stack spilling (which would require some specific compiler
>> support to mark the call).
> 
> When I first read this, I thought you were saying you changed
> posix_spawn to call plain old fork, which would have meant it would no
> longer solve Sergey's problem.  After reading over BZ#14750, I see
> that posix_spawn does still use the vfork primitive operation on
> Linux, it just does it in a lower-level manner that allows you to
> avoid the bugs you list (clone() with CLONE_VM|CLONE_VFORK and a
> temporary stack, plus blocking internal signals, which application
> code can't do for itself because we don't let it).

And my understanding it is better than the alternatives in a lot of
situations of either use of the tricky and broken vfork or the old 
fork plus exec* with its possible inherent memory issues.

> 
> But I don't see any reason we couldn't expose your bug fixes to all
> callers of vfork.  We might need to change its calling convention
> (hypothetically, pid_t nvfork(void (*proc)(void *closure))) but we
> could do that as a GNU extension.  That would fix #14750 and #10354
> for _everyone_, and changing all existing internal subprocess creation
> to use the hypothetical nvfork would be less work than changing it to
> use posix_spawn, and give a cleaner result.

I think we are discussing two slight different points here: one is to
base both popen and system internally on posix_spawn to take its advantage 
over aforementioned alternatives, and another one is to provide a vfork
like alternative as a GNU extension.

For former, by using posix_spawn internally we won't need to add *another* 
internal API which potential substantial work to accommodate some internal 
arch (for clone, stack, etc. on Linux) and system (as for hurd which does 
not use it) variants.  I still think using posix_spawn internally would 
yield simpler implementation (and other libcs, musl for instance, already 
does it).

For latter I also still think vfork-like interface are just broken without
the extra care: the vfork-like interface would require to run on its own
stack decoupled from parent or some guarantee from compiler it will create 
such stack for spill, etc. after the function call. There are other issues 
where calling functions which might alter the shared state between child and
parent are problematic (such as malloc).  In any case, such interface really 
abuses some assumptions about stack and process management, which I see it 
should be hiding behind more concise interfaces. 

> 
>> I can't really see why you think not using posix_spawn, which addresses
>> all this issues, would be better to either user a problematic interface
>> (vfork)
> 
> I don't want to encourage people to use posix_spawn because
> posix_spawn is a badly designed API.  It's difficult to use correctly.
> It's *tedious* to use correctly, which means people won't bother.  It
> can't do everything you might want to do on the child side (witness
> the discussion of adding an extension to let it do chdir()).  Its
> behavior in case of errors is underspecified.  And it might be
> implemented in terms of fork, which means it doesn't guarantee to
> solve Sergey's problem.
> 

I do agree current interface definition is limited and do not cover all
current user cases, but I do see that it does provide some improvements
over other alternatives in a lot of situations (gnome3 for instance
has started to use posix_spawn because our optimization [1]). 

Some points you raised are more a question of aesthetics than the 
interface itself, but at least posix_spawn it is designed so it can be 
extended (such as POSIX_SPAWN_SETSID recently).  I do think it would be 
better to work towards it than try to come up with some GNU specific 
interface based on a broken interface.

Right now I see two potentially extensions: one is the chdir and which
would be better its interface and another one is how to reliable close all
files.  For former I am still catching up with current discussion, but
for second I see that without help either from application or from
kernel there is not a reliable *and* race-free to close all file descriptor
with hurting process launching scalability. *BSD implementation of 
closeall, for instance, is just a dummy for over all possible FD and it
does not handle race conditions (and it is just worse than not provide
such interface IHMO).

[1] https://github.com/GNOME/glib/commit/61f54591acdfe69315cef6d1aa6d3bf1ff763082