[patch] test/leaderkill fix

[Jan Kratochvil]

On Fri, 03 Aug 2007 12:06:13 +0200, Roland McGrath wrote:
> I put this in.  I confess I didn't think too deeply about it, but what
> you've said makes sense.

I had before the attached patch which detached all the tasks first and so it
does not need the other two fixes there.  It also fixed the testcase run.

Still I believe those two fixes were right and the accepted patch is more
foolproof+simpler this way.


Thanks,
Jan
-------------- next part --------------
2007-08-01  Jan Kratochvil  <jan.kratochvil at redhat.com>

	* strace.c (handle_group_exit): Fix hang while tracing _exit(2) of
	a child with sibling(s) for TCB_ATTACHED tasks.

	* test/leaderkill2.c (start): Renamed to ...
	(start0): ... here.
	(start1): New function.
	(main): Created a new spare thread.

--- strace.c	11 Jul 2007 08:35:11 -0000	1.81
+++ strace.c	1 Aug 2007 22:03:00 -0000
@@ -2115,22 +2115,57 @@ handle_group_exit(struct tcb *tcp, int s
 		/* Mark that we are taking the process down.  */
 		tcp->flags |= TCB_EXITING | TCB_GROUP_EXITING;
 		if (tcp->flags & TCB_ATTACHED) {
-			detach(tcp, sig);
-		  	if (leader != NULL && leader != tcp) {
-				if ((leader->flags & TCB_ATTACHED) &&
-				    !(leader->flags & TCB_EXITING)) {
-					/* We need to detach the leader so
-					   that the process death will be
-					   reported to its real parent.  */
+			/* TCP->PID becomes free()d later.  */
+			int pid = tcp->pid;
+			int tcbi;
+			struct tcb *tciterp;
+			int leader_met = 0;
+
+			/* Leader must be detached as the very last task.
+			   Any of its undetached task-group children would hang
+			   us on waitpid() during the leader's detachment.
+			   Testcase: test/leaderkill2.c  */
+			for (tcbi = 0; tcbi < tcbtabsize; tcbi++) {
+				tciterp = tcbtab[tcbi];
+				if (!(tciterp->flags & TCB_INUSE))
+					continue;
+				if (!(tciterp->flags & TCB_ATTACHED))
+					continue;
+				/* Just a leftover stub TCB structure?  */
+				if ((tciterp->flags & TCB_EXITING)
+				    && tciterp != tcp)
+					continue;
+				/* Unrelated task?  We are interested in any
+				   task in a clone-relationship with TCP.
+				   Detachee itself or its leader?  */
+				if (!(tciterp == tcp || tciterp == leader
+				      || ((tciterp->flags & TCB_CLONE_THREAD)
+				  /* Child of our detachee-being-the-leader?  */
+					  && (tciterp->parent == tcp
+					    /* Sibling of our common leader?  */
+					      || tciterp->parent == leader))))
+					continue;
+				if (tciterp == leader)
+					leader_met = 1;
+				else {
 					if (debug)
 						fprintf(stderr,
-							" [%d exit %d detaches %d]\n",
-							tcp->pid, sig, leader->pid);
-					detach(leader, sig);
+							" [%d exit %d"
+							" detaches %d]\n",
+							pid, sig, tciterp->pid);
+					detach(tciterp, sig);
 				}
-				else
-					leader->flags |= TCB_GROUP_EXITING;
 			}
+			if (leader_met) {
+				if (debug)
+					fprintf(stderr,
+						" [%d exit %d"
+						" detaches leader %d]\n",
+						pid, sig, leader->pid);
+				detach(leader, sig);
+			}
+			else if (leader != NULL)
+				leader->flags |= TCB_GROUP_EXITING;
 		}
 		else if (ptrace(PTRACE_CONT, tcp->pid, (char *) 1, sig) < 0) {
 			perror("strace: ptrace(PTRACE_CONT, ...)");
--- test/leaderkill.c	5 Jul 2007 18:43:18 -0000	1.1
+++ test/leaderkill.c	1 Aug 2007 22:03:08 -0000
@@ -13,15 +13,23 @@
 #include <stdio.h>
 #include <sys/wait.h>
 
-static void *start (void *arg)
+static void *start0 (void *arg)
 {
   sleep (1);
 
   exit (42);
 }
 
+static void *start1 (void *arg)
+{
+  pause ();
+  /* NOTREACHED */
+  assert (0);
+}
+
 int main (void)
 {
+  pthread_t thread0;
   pthread_t thread1;
   int i;
   pid_t child, got_pid;
@@ -35,16 +43,13 @@ int main (void)
       case -1:
 	abort ();
       case 0:
-	i = pthread_create (&thread1, NULL, start, NULL);
+	i = pthread_create (&thread0, NULL, start0, NULL);
+	assert (i == 0);
+	i = pthread_create (&thread1, NULL, start1, NULL);
 	assert (i == 0);
-/* Two possible testcases; the second one passed even in the older versions.  */
-#if 1
 	pause ();
-#else
-	pthread_exit (NULL);
-#endif
 	/* NOTREACHED */
-	abort ();
+	assert (0);
 	break;
       default:
 	got_pid = waitpid (child, &status, 0);