Re: thread groups bug?

[David Howells]

Hi Linus,

I've attached a patch to kill all subsidiary threads in a thread group when
the main thread exits. I've made it against 2.5.6-pre1 since -pre2 fails to
compile in the IDE code somewhere.

I've tested it, and it appears to work.

Note that it sends the subsidiary threads SIGKILL with SI_DETHREAD.

Note also that subsidiary threads doing an execve() just leave the thread
group (rather than forcing the master thread to do an execve() which would be
more POSIX like).

David

diff -uNr linux-2.5.6-pre1/fs/exec.c linux-execve-256p1/fs/exec.c
--- linux-2.5.6-pre1/fs/exec.c	Wed Feb 27 08:26:55 2002
+++ linux-execve-256p1/fs/exec.c	Fri Mar  1 15:37:36 2002
@@ -513,23 +513,49 @@
 
 /*
  * An execve() will automatically "de-thread" the process.
- * Note: we don't have to hold the tasklist_lock to test
- * whether we migth need to do this. If we're not part of
- * a thread group, there is no way we can become one
- * dynamically. And if we are, we only need to protect the
- * unlink - even if we race with the last other thread exit,
- * at worst the list_del_init() might end up being a no-op.
+ * - if a master thread (PID==TGID) is doing this, then all subsidiary threads
+ *   will be killed (otherwise there will end up being two independent thread
+ *   groups with the same TGID).
+ * - if a subsidary thread is doing this, then it just leaves the thread group
  */
-static inline void de_thread(struct task_struct *tsk)
+static void de_thread(struct task_struct *tsk)
 {
-	if (!list_empty(&tsk->thread_group)) {
-		write_lock_irq(&tasklist_lock);
+	struct task_struct *sub;
+	struct list_head *head, *ptr;
+	struct siginfo info;
+	int pause;
+
+	write_lock_irq(&tasklist_lock);
+
+	if (tsk->tgid != tsk->pid) {
+		/* subsidiary thread - just escapes the group */
+		list_del_init(&tsk->thread_group);
+		tsk->tgid = tsk->pid;
+		pause = 0;
+	}
+	else {
+		/* master thread - kill all subsidiary threads */
+		info.si_signo = SIGKILL;
+		info.si_errno = 0;
+		info.si_code = SI_DETHREAD;
+		info.si_pid = current->pid;
+		info.si_uid = current->uid;
+
+		head = tsk->thread_group.next;
 		list_del_init(&tsk->thread_group);
-		write_unlock_irq(&tasklist_lock);
+
+		list_for_each(ptr,head) {
+			sub = list_entry(ptr,struct task_struct,thread_group);
+			send_sig_info(SIGKILL,&info,sub);
+		}
+
+		pause = 1;
 	}
 
-	/* Minor oddity: this might stay the same. */
-	tsk->tgid = tsk->pid;
+	write_unlock_irq(&tasklist_lock);
+
+	/* give the subsidiary threads a chance to clean themselves up */
+	if (pause) yield();
 }
 
 int flush_old_exec(struct linux_binprm * bprm)
@@ -570,7 +596,8 @@
 
 	flush_thread();
 
-	de_thread(current);
+	if (!list_empty(&current->thread_group))
+		de_thread(current);
 
 	if (bprm->e_uid != current->euid || bprm->e_gid != current->egid || 
 	    permission(bprm->file->f_dentry->d_inode,MAY_READ))
diff -uNr linux-2.5.6-pre1/include/asm-i386/siginfo.h linux-execve-256p1/include/asm-i386/siginfo.h
--- linux-2.5.6-pre1/include/asm-i386/siginfo.h	Wed Feb 27 08:27:01 2002
+++ linux-execve-256p1/include/asm-i386/siginfo.h	Fri Mar  1 15:25:47 2002
@@ -108,6 +108,7 @@
 #define SI_ASYNCIO	-4		/* sent by AIO completion */
 #define SI_SIGIO	-5		/* sent by queued SIGIO */
 #define SI_TKILL	-6		/* sent by tkill system call */
+#define SI_DETHREAD	-7		/* sent by execve() killing subsidiary threads */
 
 #define SI_FROMUSER(siptr)	((siptr)->si_code <= 0)
 #define SI_FROMKERNEL(siptr)	((siptr)->si_code > 0)