This is the mail archive of the cygwin-developers mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Expect goes crazy... spinning cpu in kill_pgrp


Dave Korn wrote:

>   So, one thing I don't know is: is it wrong for kill_pgrp to want to know
> about the ctty of an execed process, or should that data be moved into the
> leading PINFO_REDIR_SIZE part?

  Looking at the code some more, I guess in many places the answer would be to
chase the redirection to the real pid, but in this instance, where we are
going to loop over all pids anyway, there's no need; we can just skip entries
when ISSTATE(p, PID_EXECED).

  This same problem appears to affect most loops over all winpids; they
attempt to access members beyond 'exitcode' without guarding against it being
a small redirection pinfo.  I haven't seen the problem arise anywhere else
yet, expect seems to be particularly good at tripping it in this one
particular place, but it looks like it would happen if you try and nice (or
even just get the priority of) one of these execed process stubs/redirectors.
 And trying to access the /proc filesystem of one of these pids could be
expected to lead to hilarity.

  I'm experimenting with this.  It's not a full solution, it needs to do
something a bit cleverer about /proc, but it should avoid the crashes.  So no
changelog etc.; this is only FYI right now.

    cheers,
      DaveK
? winsup/cygwin/gendef.unwind
Index: winsup/cygwin/external.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/external.cc,v
retrieving revision 1.111
diff -p -u -r1.111 external.cc
--- winsup/cygwin/external.cc	14 Oct 2009 04:17:05 -0000	1.111
+++ winsup/cygwin/external.cc	21 Oct 2009 14:19:27 -0000
@@ -74,6 +74,8 @@ fillout_pinfo (pid_t pid, int winpid)
 	  ep.ctty = -1;
 	  break;
 	}
+      else if (ISSTATE(p, PID_EXECED))
+	continue;
       else if (nextpid || p->pid == pid || (winpid && thispid == (DWORD) pid))
 	{
 	  ep.ctty = p->ctty;
Index: winsup/cygwin/fhandler_termios.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler_termios.cc,v
retrieving revision 1.75
diff -p -u -r1.75 fhandler_termios.cc
--- winsup/cygwin/fhandler_termios.cc	7 Oct 2009 07:52:30 -0000	1.75
+++ winsup/cygwin/fhandler_termios.cc	21 Oct 2009 14:19:27 -0000
@@ -122,7 +122,8 @@ tty_min::kill_pgrp (int sig)
   for (unsigned i = 0; i < pids.npids; i++)
     {
       _pinfo *p = pids[i];
-      if (!p->exists () || p->ctty != ntty || p->pgid != pgid)
+      if (!p->exists () || ISSTATE(p, PID_EXECED) || p->ctty != ntty
+	  || p->pgid != pgid)
 	continue;
       if (p == myself)
 	killself++;
cvs diff: winsup/cygwin/how-crt-and-initfini.txt is a new entry, no comparison available
cvs diff: winsup/cygwin/how-cxx-abi.txt is a new entry, no comparison available
Index: winsup/cygwin/signal.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/signal.cc,v
retrieving revision 1.88
diff -p -u -r1.88 signal.cc
--- winsup/cygwin/signal.cc	9 Jun 2008 13:45:59 -0000	1.88
+++ winsup/cygwin/signal.cc	21 Oct 2009 14:19:27 -0000
@@ -284,7 +284,7 @@ kill_pgrp (pid_t pid, siginfo_t& si)
     {
       _pinfo *p = pids[i];
 
-      if (!p->exists ())
+      if (!p->exists () || ISSTATE(p, PID_EXECED))
 	continue;
 
       /* Is it a process we want to kill?  */
Index: winsup/cygwin/syscalls.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/syscalls.cc,v
retrieving revision 1.543
diff -p -u -r1.543 syscalls.cc
--- winsup/cygwin/syscalls.cc	19 Oct 2009 19:47:15 -0000	1.543
+++ winsup/cygwin/syscalls.cc	21 Oct 2009 14:19:27 -0000
@@ -3103,7 +3103,7 @@ setpriority (int which, id_t who, int va
   for (DWORD i = 0; i < pids.npids; ++i)
     {
       _pinfo *p = pids[i];
-      if (p)
+      if (p && ISSTATE(p, PID_EXECED))
 	{
 	  switch (which)
 	    {
@@ -3172,7 +3172,7 @@ getpriority (int which, id_t who)
   for (DWORD i = 0; i < pids.npids; ++i)
     {
       _pinfo *p = pids[i];
-      if (p)
+      if (p && ISSTATE(p, PID_EXECED))
 	switch (which)
 	  {
 	  case PRIO_PROCESS:

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]