Re: [ptrace] please review follow fork/exec changes

On Mon, Feb 13, 2012 at 02:04:24PM -0800, Dmitry Mikulin wrote:
> 
> 
> On 02/13/2012 07:28 AM, Konstantin Belousov wrote:
> >I looked at the orphan.patch.
> >
> >Am I right that the orphans are the real childs of the process which
> >are temporarily reparented to the debugger ? Whatever they are, a comment
> >should be added to proc.h describing what does it mean.
> 
> Done.
> 
> 
> >
> >Please provide me with a test case that demonstrates the issue
> >solved by the change.
> 
> <ftp://ftp.springdaemons.com/soft/>I attached 2 source files that compile 
> into separate binaries. scescx should be able
> The problem I'm trying to solve is to allow a parent to collect it's child 
> exit status while we're following its child. Gdb detaches from the parent 
> upon successful switch-over from parent to child. At this point due to 
> re-parenting the parent loses the child to gdb and if it's in a wait() 
> it'll get a return status that it has no children to wait for.
This text should be put somewhere in the comment. It took me some time
to re-create the reason for the patch during the read.

I will take a look at the example tomorrow, thanks.
> 
> >
> >The new LIST_FOREACH(&q->p_orphans) body is copy/pasted, together
> >with the comments, from the LIST_FOREACH(&q->p_children). Can the
> >common code be moved into some function ?
> 
> Moved the common code into a function. Didn't have time to test though.
Ok. Do not put the space between function name and '('.
Both calls to proc_to_reap() has the space.

> 
> >
> >Shouldn't there be some assertion in proc_reparent() for the case when
> >we remove child from the orphans list, that the child is no longer
> >debugged ?
> 
> Hmm... Not sure I understand...
proc_reparent() can move the child both to and from the orphan list.
If child is traced, you instert it into the orhpan list.
When removing the child from the orphan list, it means that
debugger finished with the process. My suggestion is to assert this
in proc_reparent (but I am not completely sure that this can be done
easily).

> 
> >
> >Why in proc_reparent(), in the case of P_TRACED child, you do
> >PROC_UNLOC/PROC_LOCK ?
> 
> No idea how it ended up like that... I'll clean it up.
> 
> >
> >It seems that now wait4(2) can be called from the real (non-debugger)
> >parent first and result in the call to proc_reap(), isn't it ? We would
> >then just reparent the child back to the caller, still leaving the
> >zombie and confusing debugger.
> When either gdb or the real parent gets to proc_reap() the process wouldn't 
> get destroyed, it'll get caught by the following clause:
>     if (p->p_oppid && (t = pfind(p->p_oppid)) != NULL) {
> 
> and the real parent with get the child back into the children's list while 
> gdb will get it into the orphan list. The second time around when 
> proc_reap() is entered, p->p_oppid will be 0 and the process will get 
> really reaped. Does it make sense? And proc_reparent() attempts to keep the 
> orphan list clean and not have the same entries and the list of siblings.
Right, this is what I figured. But I asked about some further implication
of this change:

if real parent spuriosly calls wait4(2) on the child pid after the child
exited, but before the debugger called the wait4(), then exactly the
code you noted above will be run. This results in the child being fully
returned to the original parent.

Next, the wait4() call from debugger gets an error, and zombie will be
kept around until parent calls wait4() for this pid once more.

Am I missed something ?
> 
> The idea is to keep the real parent alive ling enough  to collect the 
> statuc of the child running under gdb.
>