[PATCH] posix: Fix improper assert in Linux posix_spawn (BZ#22273)

[Adhemerval Zanella]

On 18/10/2017 12:12, Florian Weimer wrote:
> On 10/18/2017 01:51 PM, Adhemerval Zanella wrote:
>> +Â Â Â  /* There still an unlikely case where the child is cancelled after
>> +       setting args.to a positive error value.  Also due a possible
>> +Â Â Â Â Â Â  pid reuse race (where the kernel allocated the same pid to
>> +Â Â Â Â Â Â  unrelated process) we need not to undefinitely hang expecting
>> +       an invalid pid.  In both cases an error is returned to the
>> +       caller.  */
>> +Â Â Â  __waitpid (new_pid, NULL, WNOHANG);
> 
> It took a while to wrap my head around this one.  I don't think the WNOHANG makes much of a difference because in the non-race case, the kernel should ensure that the wait notification is ready before vfork returns from the kernel in the parent process.  We had some issues with wait notifications, but I hope this one is actually properly serialized.

Indeed WNOHANG only helps for the race case to just avoid posix_spawn
hang indefinitely (which should be quite rare).  And I am not aware of
any synchronization issues when calling clone (CLONE_VFORK) plus 
waitpid (WNOHANG). I tried some stress run on both kernel 3.2 and
4.4 to check if waitpid would fail in this scenario and it seems to
behave as expected.

> 
> The race case (i.e., PID reuse because a signal handler or another thread does a wildcard waitpid) is vaguely undefined anyway.
> 
> We could probably do better if we called clone without SIGCHLD.  Not sure if that works, but it's a different enhancement and out of scope for this patch.  If Andreas is happy with what you've got, so am I.

I do not think this scenario would help much because caller can still
use a child handler using Linux only flags (__WALL or __WCLONE).

I will commit the patch with the types/grammar mistakes raised by
Andreas.