2004-06-30 06:00:30

by Andrea Arcangeli

[permalink] [raw]
Subject: zombie with CLONE_THREAD

Hello,

I debugged a problem with CLONE_THREAD under strace generating zombies
that cannot be reaped by init.

Basically what's going on is that release_task is never called on the
clones, and in turn the parent thread will remain zombie forever because
thread_group_empty == 0 (it never notifies init). the group can become
empty only after release_task has been called on all the clones.

What's going on is that if the clone happen to be under strace by the
time it exits its state will not be set to TASK_DEAD and nobody will
ever call wait4 on the clone because the parent is being killed at the
same time. But the parent cannot go away until the clone goes away too.
I believe strace needs as well a little race where it has the sigchld
disabled but what I'm discussing here is still a kernel bug generating
zombie threads.

I think I could have fixed even with a strictier patch (adding a
exit_signal == -1 check just to cover that case), but I believe that it
makes no sense to leave ptrace enabled on a clone that is being killed,
it happens to be safe without a thread-group just because there will be
always init able to call wait4->release_task on it, that will call
ptrace_unlink later in release_task, same goes for the "leader" of the
thread group, that as well can be detached by ptrace via release_task).

so far this thing worked fine in practice. Would be nice to hear a
comment from somebody who understand this CLONE_THREAD signal
mess^Wstuff with ptrace better.

--- sles/kernel/exit.c.~1~ 2004-06-30 01:41:58.000000000 +0200
+++ sles/kernel/exit.c 2004-06-30 07:49:21.705154000 +0200
@@ -734,6 +734,13 @@ static void exit_notify(struct task_stru
do_notify_parent(tsk, SIGCHLD);
}

+ /*
+ * To allow the group leader of a thread group to be released
+ * we must really go away synchronously if exit_signal == -1.
+ */
+ if (unlikely(tsk->ptrace) && tsk != tsk->group_leader)
+ __ptrace_unlink(tsk);
+
state = TASK_ZOMBIE;
if (tsk->exit_signal == -1 && tsk->ptrace == 0)
state = TASK_DEAD;


2004-06-30 06:09:09

by Andrew Morton

[permalink] [raw]
Subject: Re: zombie with CLONE_THREAD

Andrea Arcangeli <[email protected]> wrote:
>
> Hello,
>
> I debugged a problem with CLONE_THREAD under strace generating zombies
> that cannot be reaped by init.
>
> Basically what's going on is that release_task is never called on the
> clones, and in turn the parent thread will remain zombie forever because
> thread_group_empty == 0 (it never notifies init). the group can become
> empty only after release_task has been called on all the clones.
>
> What's going on is that if the clone happen to be under strace by the
> time it exits its state will not be set to TASK_DEAD and nobody will
> ever call wait4 on the clone because the parent is being killed at the
> same time. But the parent cannot go away until the clone goes away too.
> I believe strace needs as well a little race where it has the sigchld
> disabled but what I'm discussing here is still a kernel bug generating
> zombie threads.
>
> I think I could have fixed even with a strictier patch (adding a
> exit_signal == -1 check just to cover that case), but I believe that it
> makes no sense to leave ptrace enabled on a clone that is being killed,
> it happens to be safe without a thread-group just because there will be
> always init able to call wait4->release_task on it, that will call
> ptrace_unlink later in release_task, same goes for the "leader" of the
> thread group, that as well can be detached by ptrace via release_task).
>
> so far this thing worked fine in practice. Would be nice to hear a
> comment from somebody who understand this CLONE_THREAD signal
> mess^Wstuff with ptrace better.

Maybe Linus or Roland could comment.

> --- sles/kernel/exit.c.~1~ 2004-06-30 01:41:58.000000000 +0200
> +++ sles/kernel/exit.c 2004-06-30 07:49:21.705154000 +0200
> @@ -734,6 +734,13 @@ static void exit_notify(struct task_stru
> do_notify_parent(tsk, SIGCHLD);
> }
>
> + /*
> + * To allow the group leader of a thread group to be released
> + * we must really go away synchronously if exit_signal == -1.
> + */
> + if (unlikely(tsk->ptrace) && tsk != tsk->group_leader)
> + __ptrace_unlink(tsk);
> +

Should this be using thread_group_leader()?

2004-06-30 07:14:12

by Roland McGrath

[permalink] [raw]
Subject: Re: zombie with CLONE_THREAD

That change sure looks incorrect and unnecessary to me. It wasn't clear to
me from Andrea's description whether there is really any kernel bug here or
not. If some process is ptrace'ing a CLONE_THREAD thread, then the thread
should not disappear as this patch makes it do. It becomes a zombie and
reports its death status to the ptracer. When the ptracer waits for it, it
reverts to its original parent and then the logic in wait_task_zombie
should call release_task for the CLONE_THREAD case.

Are you saying that if the ptracer dies, it can leave some threads in limbo?
I think that case is supposed to work because forget_original_parent will
move all the threads ptrace'd by the dying tracer process to be ptrace'd by
init, which will then clean up their zombies as previously described.


Thanks,
Roland

2004-06-30 09:04:51

by Andreas Schwab

[permalink] [raw]
Subject: Re: zombie with CLONE_THREAD


Andreas.

--
Andreas Schwab, SuSE Labs, [email protected]
SuSE Linux AG, Maxfeldstra?e 5, 90409 N?rnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."


Attachments:
(No filename) (551.00 B)
clone.c (966.00 B)
Download all attachments