* 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 5bfd7db6a0660f7de8f932efd56f730f5dad7bec..0102ce87ef9dac5cadd5c8f11a852763f8dbff99 100644 (file)
--- 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 03cd251093925b7c345363ae2851c5e6e4ac1fa3..7107361a7798b43e615519dc35f5148d46028425 100644 (file)
--- 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 57945db7b281dc6a0efe69fd30c455022f611f3e..80d468a29d035b26b7e479435501672c5b5fbb89 100644 (file)
--- 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;