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: [RFC] Fix expect leaky pipes


On Mon, May 29, 2006 at 12:00:05AM +0100, Dave Korn wrote:
>
>    Hi everyone,
>
>
>  I've been looking at the problem with expect leaking pipe handles and
>running dog-slow as its handle table fills up (or on my old w2k box, causing
>a complete kernel lock-up), and I *think* I've got it.  After adding a whole
>load of termios_printfs to track everything that was going on, it seems like
>there's just an ordering problem in the shutdown of the final master.  In
>fhandler_pty_master::close we check whether the master is still alive, and
>if so, only call the fhandler_tty_common::close routine, but if the master
>is no longer alive (and I /think/ that means the same as "if we're closing
>the final handle to it"), we close the from/to master pipes as well.
>
>  Now, in expect what happens is that when the child processes have all
>exited and closed their last open handles to the tty, the expect master
>tries to close its fd to the pty master.  On entry the master is still
>alive (signified by the respective event existing) but when we call the
>common::close handler it closes the last open handle to the 'master alive'
>event which then goes away.  So we've closed the last fd to it but haven't
>closed the from/to master pipes, and that's how they leak.  I've fixed the
>problem by re-checking after calling out to the common::close and seeing if
>the master is *now* dead, and closing the master pipes at that time.
>
>  Anyway, this isn't quite right, but I need to ask some advice: initially,
>we had (modulo #if-0 and whitespace)
>
>
>int
>fhandler_pty_master::close ()
>{
>  if (get_ttyp ()->master_alive ())
>    fhandler_tty_common::close ();
>  else
>    {
>      termios_printf ("freeing tty%d (%d)", get_unit (), get_ttyp ()->ntty);
>      if (get_ttyp ()->from_master)
>	CloseHandle (get_ttyp ()->from_master);
>      if (get_ttyp ()->to_master)
>	CloseHandle (get_ttyp ()->to_master);
>      fhandler_tty_common::close ();
>      if (!hExeced)
>	get_ttyp ()->init ();
>    }
>  return 0;
>}
>
>and I've changed that to
>
>int
>fhandler_pty_master::close ()
>{
>  if (get_ttyp ()->master_alive ())
>    fhandler_tty_common::close ();
>  /* That could have closed the last master, so re-check.  */
>  if (!get_ttyp ()->master_alive ())
>    {
>      termios_printf ("freeing tty%d (%d)", get_unit (), get_ttyp ()->ntty);
>      if (get_ttyp ()->from_master)
>	CloseHandle (get_ttyp ()->from_master);
>      if (get_ttyp ()->to_master)
>	CloseHandle (get_ttyp ()->to_master);
>      fhandler_tty_common::close ();
>      if (!hExeced)
>	get_ttyp ()->init ();
>    }
>  return 0;
>}

I think it should be ok to just do:

  fhandler_tty_common::close ();

  if (!get_ttyp ()->master_alive ())
      {
	termios_printf ("freeing tty%d (%d)", get_unit (), get_ttyp ()->ntty);
	if (get_ttyp ()->from_master)
	 CloseHandle (get_ttyp ()->from_master);
	if (get_ttyp ()->to_master)
	 CloseHandle (get_ttyp ()->to_master);
        ...

cgf


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