Re: eligible_child() && __WCLONE && task_detached() (Was: mmotm 2009-08-24-16-24 uploaded)

[Roland McGrath <roland@redhat.com>]

> eligible_child:
> 
> 	/* Wait for all children (clone and not) if __WALL is set;
> 	 * otherwise, wait for clone children *only* if __WCLONE is
> 	 * set; otherwise, wait for non-clone children *only*.  (Note:
> 	 * A "clone" child here is one that reports to its parent
> 	 * using a signal other than SIGCHLD.) */
> 
> 	if (((p->exit_signal != SIGCHLD) ^ !!(wo->wo_flags & __WCLONE))
> 	    && !(wo->wo_flags & __WALL))
> 		return 0;
> 
> I just can't understand what is the supposed behaviour when p is
> sub-thread and p->exit_signal == -1.

As you say, you're not even supposed to be here when exit_signal = -1,
except for the ptrace case.  This logic exists for the non-CLONE_THREAD
clone case, i.e. ->exit_signal != SIGCHLD and != -1.

> IOW, perhaps this check should be
> 
> 	if (!task_detached(p) && !(wo->wo_flags & __WALL) &&
> 	    (p->exit_signal != SIGCHLD) ^ !!(wo->wo_flags & __WCLONE))
> 		return 0;

That seems OK to me.

> When task_detached(p) == T, "p->exit_signal != SIGCHLD" looks like a
> false positive to me. Because -1 is not a siganl, this is a marker
> which indicates the deatached task - sub-thread or EXIT_DEAD. It _seems_
> to me this check was added when threads were processes, and it was
> possible to wait/reap a thread, not process.

That's all still possible, it's just no longer common.
i.e. clone(SIGUSR1|CLONE_VM, ...) or whatnot (no CLONE_THREAD).

> In short. If ptracer calls wait4(ptraced_sub_thread), is it really
> supposed it must use __WCLONE || __WALL?

I suspect not, but I'm not quite sure.  That is, it makes sense to me that
a ptracer should always get its tracees in all waits.  That is consistent
with not having to use WUNTRACED, for example.  But I'm not really sure any
more what the historical behavior of this has been.  The code in GDB uses
__WALL and __WCLONE in various places, clearly assuming that just ptrace
alone is not enough for every wait4 call to catch every tracee.  It seems
likely that this method was necessary in the past and that's why the code
is like that.


Thanks,
Roland