From: Denys Vlasenko <dvlasenk@redhat.com>
Date: Sat, 17 Jan 2009 00:21:31 +0000 (+0000)
Subject: * process.c: Add a comment. No code changes.
X-Git-Tag: v4.5.19~98
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f9a7e63a1ad57a87c1e6be10b7a6b49282ed1203;p=strace

* process.c: Add a comment. No code changes.
* strace.c (collect_stopped_tcbs): Stop reversing list of stopped
tcp's. I'm not totally convinced it is crucial, but this is surely
fits the concept of "least surprise".
Do not collect TCB_SUSPENDED tcp's (this is closer to how
it was before).
(handle_stopped_tcbs): Remove the code to reject TCB_SUSPENDED tcp's,
it's done earlier now. In an unobvious way, this was causing
SIGSTOPs from freshly attached children to be misinterpreted.
---

diff --git a/ChangeLog b/ChangeLog
index 5bfd7db6..0102ce87 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2009-01-17  Denys Vlasenko  <dvlasenk@redhat.com>
+
+	* process.c: Add a comment. No code changes.
+	* strace.c (collect_stopped_tcbs): Stop reversing list of stopped
+	tcp's. I'm not totally convinced it is crucial, but this is surely
+	fits the concept of "least surprise".
+	Do not collect TCB_SUSPENDED tcp's (this is closer to how
+	it was before).
+	(handle_stopped_tcbs): Remove the code to reject TCB_SUSPENDED tcp's,
+	it's done earlier now. In an unobvious way, this was causing
+	SIGSTOPs from freshly attached children to be misinterpreted.
+
 2009-01-14  Denys Vlasenko  <dvlasenk@redhat.com>
 
 	* linux/bfin/syscallent.h: sys_futex has 6 parameters, not 5.
diff --git a/process.c b/process.c
index 03cd2510..7107361a 100644
--- a/process.c
+++ b/process.c
@@ -963,6 +963,9 @@ struct tcb *tcp;
 				clearbpt(tcpchild);
 
 			tcpchild->flags &= ~(TCB_SUSPENDED|TCB_STARTUP);
+			/* TCB_SUSPENDED tasks are not collected by waitpid
+			 * loop, and left stopped. Restart it:
+			 */
 			if (ptrace_restart(PTRACE_SYSCALL, tcpchild, 0) < 0)
 				return -1;
 
diff --git a/strace.c b/strace.c
index 57945db7..80d468a2 100644
--- a/strace.c
+++ b/strace.c
@@ -2279,7 +2279,8 @@ collect_stopped_tcbs(void)
 	struct rusage ru;
 	struct rusage* ru_ptr = cflag ? &ru : NULL;
 	int wnohang = 0;
-	struct tcb *found_tcps = NULL;
+	struct tcb *found_tcps;
+	struct tcb **nextp = &found_tcps;
 #ifdef __WALL
 	int wait4_options = __WALL;
 #endif
@@ -2401,10 +2402,28 @@ Process %d attached (waiting for parent)\n",
 			tcp->stime = ru.ru_stime;
 #endif
 		}
+		if (tcp->flags & TCB_SUSPENDED) {
+			/*
+			 * Apparently, doing any ptrace() call on a stopped
+			 * process, provokes the kernel to report the process
+			 * status again on a subsequent wait(), even if the
+			 * process has not been actually restarted.
+			 * Since we have inspected the arguments of suspended
+			 * processes we end up here testing for this case.
+			 */
+			continue;
+		}
+
 		tcp->wait_status = status;
 #ifdef LINUX
-		tcp->next_need_service = found_tcps;
-		found_tcps = tcp;
+		/* It is important to not invert the order of tasks
+		 * to process. For one, alloc_tcb() above picks newly forked
+		 * threads in some order, processing of them and their parent
+		 * should be in the same order, otherwise bad things happen
+		 * (misinterpreted SIGSTOPs and such).
+		 */
+		*nextp = tcp;
+		nextp = &tcp->next_need_service;
 		wnohang = WNOHANG;
 #endif
 #ifdef SUNOS4
@@ -2413,9 +2432,10 @@ Process %d attached (waiting for parent)\n",
 		 */
 		break;
 #endif
-	} /* while (1) - collecting all stopped/exited tracees */
+	}
 
-	return tcp;
+	*nextp = NULL;
+	return found_tcps;
 }
 
 static int
@@ -2425,18 +2445,6 @@ handle_stopped_tcbs(struct tcb *tcp)
 		int pid;
 		int status;
 
-		if (tcp->flags & TCB_SUSPENDED) {
-			/*
-			 * Apparently, doing any ptrace() call on a stopped
-			 * process, provokes the kernel to report the process
-			 * status again on a subsequent wait(), even if the
-			 * process has not been actually restarted.
-			 * Since we have inspected the arguments of suspended
-			 * processes we end up here testing for this case.
-			 */
-			continue;
-		}
-
 		outf = tcp->outf;
 		status = tcp->wait_status;
 		pid = tcp->pid;