Re: [RFC v3 03/23] sysdeps/wait: Use waitid if avaliable

[Rich Felker <dalias at libc dot org>]

On Sun, Jul 21, 2019 at 04:40:27PM -0500, Eric W. Biederman wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > On Sun, Jul 21, 2019 at 8:45 AM Eric W. Biederman <ebiederm@xmission.com> wrote:
> >>
> >> We either add P_PROCESS_PGID to the kernel's waitid or we add wait4.
> >
> > Do we actually need a new flag?
> >
> > Why not just allow people to pass in pid = 0 with the existing P_PGID flag?
> >
> > Those are the traditional and documented waitpid() semantics. Why
> > wouldn't we use those semantics for waitid() too?
> >
> > And since our current (broken) waitid() returns EINVAL for that case,
> > it's even trivial to test for in user space (and fall back on the
> > broken racy code in user space - which you have to do anyway).
> 
> Our current waitid implementation when serving as waitid is not broken.
> 
> What is broken is the waitpid emulation on top of waitid.
> 
> The way waitid is defined waitid P_PGID with a pid of 0 means
> wait for the process group id of 0.  Which in theory and in limited
> practice exists.  AKA that is the pid and process group of the idle
> thread.
> 
> Further there is the outstanding question.  Should P_PROCESS_PGID refer
> to the curent processes process group id at the time of the waitid call,
> or should P_PROCESS_PGID refer to the process group id when a child is
> found?
> 
> It has been suggested elswehere in this conversation that 161550d74c07
> ("pid: sys_wait... fixes") may have introduced a regression.  If so
> the current wait4 behavior of capturing the process group at the time
> of call is wrong and needs to be fixed.
> 
> Not capuring the pid at time time of call is a very different behavior
> of P_PROCESS_PGID vs all of the other waitid cases which do have the pid
> fixed at the time of the call which further argues a separate flag
> because the behavior would be distinctly different.

I believe that is a regression, and that the "capturing it" line of
thinking is misleading. It's thinking in terms of implementation
rather than interface contract. The interface contract covers which
children it can reap and when it can block. If the call remains
blocking, there must be a possible ordering of events, with no
observable effects contradicting that order, by which there are no
reapable children in the process's current pgid. If the call returns
successfully, there must be a possible ordering of events, with no
observable effects contradicting that order, by which the caller's
pgid and the reaped process's pgid were equal.

> > Honestly, that seems like the simplest soilution, but also like the
> > only sane model. The fact that P_PGID with a zero pid doesn't work
> > seems to simply be a bug right now, and keeps waitid() from being a
> > proper superset of waitpid().
> 
> In the context of waitid it does not look sane.  Unlike the other wait
> functions waitid does not have any magic ids and by having an idtype
> field makes that kind of magic unnecessary.  Further it is trivial
> to add one line to the architecture independent enumeration and have 0
> risk of confusion or of regressing user space.

I'm in agreement that if an extension to the waitid syscall is added,
it should be P_PROCESS_PGID, not defining some special case for
pid==0. It's not entirely clear but arguable that the standard
requires EINVAL for P_PGID + pid==0, and plausible that there are
applications that depend on this. We could emulate the EINVAL case in
userspace, but assigning weird semantics to special cases is just a
mess of potential future headaches when it would be trivial to do it
right. And doing it right would also make programming userspace side
easier.

Rich