On 06/05/2019 09:50, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> On 04/05/2019 12:59, Paul Eggert wrote:
>>> Florian Weimer wrote:
>>>
>>>> doing this is worthwhile because
>>>> it increases test coverage for our vfork implementation. I think we
>>>> need to provide a working vfork.
>>>
>>> Yes; regardless of whether we use vfork to implement posix_spawn, too many programs rely on vfork to withdraw it any time in the foreseeable future.
>>
>> That does not mean we should *promote* the interface, which has a lot
>> of downsides and are tricky to use correctly. If we do want to support
>> a vfork-like interface I would work toward Zack Weinberg suggestion to add
>> a callback 'pre-fork' routine and extend it to make it possible to user
>> supply a stack.
>>
>> Something like:
>>
>> pid_t xfork (void *(*pre_fork (void *), void *stack, size_t stacksize);
>>
>> It will make the required libc handling (disable asynchronous cancellation,
>> block all signals, call the syscall primitive with required flags, restore
>> signal handling, re-enable asynchronous cancellation, handle error state).
>> The stack is optional and using NULL will make it use a automatic allocate
>> one (not sure if issuing a VLA is worth based on stacksize or use a
>> pre-defined value).
>
> If we want to go this route, I'd suggest something like this as a
> building block:
>
> pid_t clone_samestack (unsigned int flags, void (*action) (void *)
> void *closure);
>
> and it would be a fatal error if ACTION (CLOSURE) returns. The function
> would fail with EINVAL if FLAGS contained CLONE_VM without CLONE_VFORK.
>
> Once we have that, the need for explicit stack management goes away.
But would it just a shim wrapper over clone (to handle the required kABI)
as vfork currently is or something more sane as posix_spawn to work
correctly along libc? Also, would be just an internal interface or
a possible extension?
Because for a clone wrapper, I think would be simpler to just make using
clone instead by allowing it accept a null stack input.
---
struct wrapper_argument
{
void (*action) (void *) fn;
void *arg;
};
void wrapper (void *input)
{
struct wrapper_argument *arg = input;
arg->fn (arg->arg);
abort ();
}
pid_t clone_samestack (unsigned int flags, void (*action) (void *)
void *closure)
{
int clone_flags = CLONE_VM | CLONE_VFORK | SIGCHLD;
#ifndef __ia64__
return clone (wrapper, NULL, clone_flags,
&(struct wrapper_argument) { action, closure }));
#else
return __clone2 (wrapper, NULL, sizeof stack,
&(struct wrapper_argument) { action, closure }));
#endif
}
---
It also wouldn't require potentially another arch-specific assembly
implementation.
However my main issue with such interface where is reuses the stack from
parent process is how safe this construction is. That's why I think the
xfork symbol, which either allocates a pre-defined stack or accepts a
user-define one (as for sigaltstack) should be safer and play along
libc interfaces better (it could either share some code with posix_spawn).