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.
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.
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...
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.
The idea is to keep the real parent alive ling enough to collect the statuc of
the child running under gdb.
Index: sys/proc.h
===================================================================
--- sys/proc.h (revision 231328)
+++ sys/proc.h (working copy)
@@ -507,8 +507,6 @@ struct proc {
/* The following fields are all zeroed upon creation in fork. */
#define p_startzero p_oppid
pid_t p_oppid; /* (c + e) Save ppid in ptrace. XXX */
- int p_dbg_child; /* (c + e) # of debugged children in
- ptrace. */
struct vmspace *p_vmspace; /* (b) Address space. */
u_int p_swtick; /* (c) Tick when swapped in or out. */
struct itimerval p_realtimer; /* (c) Alarm timer. */
@@ -576,6 +574,11 @@ struct proc {
after fork. */
uint64_t p_prev_runtime; /* (c) Resource usage accounting. */
struct racct *p_racct; /* (b) Resource accounting. */
+ /* An orphan is process that has beed re-parented to the debugger
+ as a result of attaching to it. Need to keep track of tham to
+ be able to collect the exit status of what used to be children. */
+ LIST_ENTRY(proc) p_orphan; /* (e) List of orphan processes. */
+ LIST_HEAD(, proc) p_orphans; /* (e) Pointer to list of orphans. */
};
#define p_session p_pgrp->pg_session
@@ -867,7 +870,7 @@ void procinit(void);
void proc_linkup0(struct proc *p, struct thread *td);
void proc_linkup(struct proc *p, struct thread *td);
void proc_reap(struct thread *td, struct proc *p, int *status, int options,
- struct rusage *rusage);
+ struct rusage *rusage, int child);
void proc_reparent(struct proc *child, struct proc *newparent);
struct pstats *pstats_alloc(void);
void pstats_fork(struct pstats *src, struct pstats *dst);
Index: kern/kern_exit.c
===================================================================
--- kern/kern_exit.c (revision 231328)
+++ kern/kern_exit.c (working copy)
@@ -680,7 +680,7 @@ sys_wait4(struct thread *td, struct wait_args *uap
*/
void
proc_reap(struct thread *td, struct proc *p, int *status, int options,
- struct rusage *rusage)
+ struct rusage *rusage, int child)
{
struct proc *q, *t;
@@ -720,7 +720,6 @@ proc_reap(struct thread *td, struct proc *p, int *
if (p->p_oppid && (t = pfind(p->p_oppid)) != NULL) {
PROC_LOCK(p);
proc_reparent(p, t);
- p->p_pptr->p_dbg_child--;
p->p_oppid = 0;
PROC_UNLOCK(p);
pksignal(t, SIGCHLD, p->p_ksi);
@@ -738,7 +737,10 @@ proc_reap(struct thread *td, struct proc *p, int *
sx_xlock(&allproc_lock);
LIST_REMOVE(p, p_list); /* off zombproc */
sx_xunlock(&allproc_lock);
- LIST_REMOVE(p, p_sibling);
+ if (child)
+ LIST_REMOVE(p, p_sibling);
+ else
+ LIST_REMOVE(p, p_orphan);
leavepgrp(p);
#ifdef PROCDESC
if (p->p_procdesc != NULL)
@@ -803,12 +805,54 @@ proc_reap(struct thread *td, struct proc *p, int *
sx_xunlock(&allproc_lock);
}
+static int
+proc_to_reap (struct thread *td, struct proc *p, pid_t pid, int *status,
+ int options, struct rusage *rusage, int child)
+{
+ struct proc *q;
+
+ q = td->td_proc;
+ PROC_LOCK(p);
+ if (pid != WAIT_ANY &&
+ p->p_pid != pid && p->p_pgid != -pid) {
+ PROC_UNLOCK(p);
+ return (0);
+ }
+ if (p_canwait(td, p)) {
+ PROC_UNLOCK(p);
+ return (0);
+ }
+
+ /*
+ * This special case handles a kthread spawned by linux_clone
+ * (see linux_misc.c). The linux_wait4 and linux_waitpid
+ * functions need to be able to distinguish between waiting
+ * on a process and waiting on a thread. It is a thread if
+ * p_sigparent is not SIGCHLD, and the WLINUXCLONE option
+ * signifies we want to wait for threads and not processes.
+ */
+ if ((p->p_sigparent != SIGCHLD) ^
+ ((options & WLINUXCLONE) != 0)) {
+ PROC_UNLOCK(p);
+ return (0);
+ }
+
+ PROC_SLOCK(p);
+ if (p->p_state == PRS_ZOMBIE) {
+ proc_reap(td, p, status, options, rusage, child);
+ return (-1);
+ }
+ PROC_SUNLOCK(p);
+ PROC_UNLOCK(p);
+ return (1);
+}
+
int
kern_wait(struct thread *td, pid_t pid, int *status, int options,
struct rusage *rusage)
{
struct proc *p, *q;
- int error, nfound;
+ int error, nfound, ret;
AUDIT_ARG_PID(pid);
AUDIT_ARG_VALUE(options);
@@ -831,37 +875,16 @@ loop:
nfound = 0;
sx_xlock(&proctree_lock);
LIST_FOREACH(p, &q->p_children, p_sibling) {
- PROC_LOCK(p);
- if (pid != WAIT_ANY &&
- p->p_pid != pid && p->p_pgid != -pid) {
- PROC_UNLOCK(p);
+ ret = proc_to_reap (td, p, pid, status, options, rusage, 1);
+ if (ret == 0)
continue;
- }
- if (p_canwait(td, p)) {
- PROC_UNLOCK(p);
- continue;
- }
+ else if (ret == 1)
+ nfound++;
+ else
+ return (0);
- /*
- * This special case handles a kthread spawned by linux_clone
- * (see linux_misc.c). The linux_wait4 and linux_waitpid
- * functions need to be able to distinguish between waiting
- * on a process and waiting on a thread. It is a thread if
- * p_sigparent is not SIGCHLD, and the WLINUXCLONE option
- * signifies we want to wait for threads and not processes.
- */
- if ((p->p_sigparent != SIGCHLD) ^
- ((options & WLINUXCLONE) != 0)) {
- PROC_UNLOCK(p);
- continue;
- }
-
- nfound++;
+ PROC_LOCK(p);
PROC_SLOCK(p);
- if (p->p_state == PRS_ZOMBIE) {
- proc_reap(td, p, status, options, rusage);
- return (0);
- }
if ((p->p_flag & P_STOPPED_SIG) &&
(p->p_suspcount == p->p_numthreads) &&
(p->p_flag & P_WAITED) == 0 &&
@@ -893,16 +916,21 @@ loop:
if (status)
*status = SIGCONT;
- return (0);
}
PROC_UNLOCK(p);
}
+ LIST_FOREACH(p, &q->p_orphans, p_orphan) {
+ ret = proc_to_reap (td, p, pid, status, options, rusage, 0);
+ if (ret == 0)
+ continue;
+ else if (ret == 1)
+ nfound++;
+ else
+ return (0);
+ }
if (nfound == 0) {
sx_xunlock(&proctree_lock);
- if (td->td_proc->p_dbg_child)
- return (0);
- else
- return (ECHILD);
+ return (ECHILD);
}
if (options & WNOHANG) {
sx_xunlock(&proctree_lock);
@@ -929,6 +957,7 @@ loop:
void
proc_reparent(struct proc *child, struct proc *parent)
{
+ struct proc *p;
sx_assert(&proctree_lock, SX_XLOCKED);
PROC_LOCK_ASSERT(child, MA_OWNED);
@@ -940,5 +969,15 @@ proc_reparent(struct proc *child, struct proc *par
PROC_UNLOCK(child->p_pptr);
LIST_REMOVE(child, p_sibling);
LIST_INSERT_HEAD(&parent->p_children, child, p_sibling);
+
+ LIST_FOREACH(p, &parent->p_orphans, p_orphan) {
+ if (p == child) {
+ LIST_REMOVE(child, p_orphan);
+ break;
+ }
+ }
+ if (child->p_flag & P_TRACED)
+ LIST_INSERT_HEAD(&child->p_pptr->p_orphans, child, p_orphan);
+
child->p_pptr = parent;
}
Index: kern/kern_fork.c
===================================================================
--- kern/kern_fork.c (revision 231328)
+++ kern/kern_fork.c (working copy)
@@ -590,6 +590,7 @@ do_fork(struct thread *td, int flags, struct proc
LIST_INSERT_AFTER(p1, p2, p_pglist);
PGRP_UNLOCK(p1->p_pgrp);
LIST_INIT(&p2->p_children);
+ LIST_INIT(&p2->p_orphans);
callout_init(&p2->p_itcallout, CALLOUT_MPSAFE);
Index: kern/sys_process.c
===================================================================
--- kern/sys_process.c (revision 231328)
+++ kern/sys_process.c (working copy)
@@ -841,8 +841,6 @@ kern_ptrace(struct thread *td, int req, pid_t pid,
p->p_flag |= P_TRACED;
p->p_oppid = p->p_pptr->p_pid;
if (p->p_pptr != td->td_proc) {
- /* Remember that a child is being debugged(traced). */
- p->p_pptr->p_dbg_child++;
proc_reparent(p, td->td_proc);
}
data = SIGSTOP;
@@ -931,7 +929,6 @@ kern_ptrace(struct thread *td, int req, pid_t pid,
PROC_UNLOCK(pp);
PROC_LOCK(p);
proc_reparent(p, pp);
- p->p_pptr->p_dbg_child--;
if (pp == initproc)
p->p_sigparent = SIGCHLD;
}
Index: kern/sys_procdesc.c
===================================================================
--- kern/sys_procdesc.c (revision 231328)
+++ kern/sys_procdesc.c (working copy)
@@ -367,7 +367,7 @@ procdesc_close(struct file *fp, struct thread *td)
* procdesc_reap().
*/
PROC_SLOCK(p);
- proc_reap(curthread, p, NULL, 0, NULL);
+ proc_reap(curthread, p, NULL, 0, NULL, 0);
} else {
/*
* If the process is not yet dead, we need to kill it, but we
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[email protected]"
