[PATCH] Fix for sending SIGHUP when the pty master side is close()-ed

Pavel Tsekov ptsekov@gmx.net
Wed Aug 17 15:29:00 GMT 2005


Hello,

Today, I've finally tracked down the cause of the problem with the stale
subshells on MC exit. In the past I've tried to attack the problem in a
different manner and wasn't looking at the right place - I suspected some
kind of problem / race in the signal handling / delivery, but I was wrong.

Here is a short description of the problem for those who forgot the
details or never knew of its existance.

MC has the ability to spawn a shell and to allow the user to switch
between the MC's ui and this subshell using a keystroke (Ctrl + O).
When in the subshell the user can do anything he/she can do whith a normal
shell. Now, MC doesn't do any special cleanup related to the subshell -
it relies on the fact that when close() is called on a pty master end a
SIGHUP will be send to the app on the other end. Unfortunately sending the
SIGHUP signal doesn't work very well on Cygwin and the subshell started by
MC is rarely killed.

The following code from fhandler_tty_common::close() is responsible for
initiating the sending of a SIGHUP signal in the situation described
above:

[...]
  /* Send EOF to slaves if master side is closed */
  if (!get_ttyp ()->master_alive ())
    {
      termios_printf ("no more masters left. sending EOF");
      SetEvent (input_available_event);
    }
[...]
  termios_printf ("CloseHandle (output_mutex<%p>), %E", output_mutex);
  if (!ForceCloseHandle1 (get_handle (), from_pty))
    termios_printf ("CloseHandle (get_handle ()<%p>), %E", get_handle ());
  if (!ForceCloseHandle1 (get_output_handle (), to_pty))
    termios_printf ("CloseHandle (get_output_handle ()<%p>), %E", get_output_handle ());
[...]

The first part of the code will notify the slave that there is pending
input actively forcing it to read data from the master. The
second part of the code will close the the pipes used to talk to the pty
end. Both parts work together to make the slave pty think that there is
EOF condition. The following lines from fhandler_pty_master::close() are
also of interest:

[...]
  if (!get_ttyp ()->master_alive ())
    {
      termios_printf ("freeing tty%d (%d)", get_unit (), get_ttyp
()->ntty);
#if 0
      if (get_ttyp ()->to_slave)
        ForceCloseHandle1 (get_ttyp ()->to_slave, to_slave);
      if (get_ttyp ()->from_slave)
        ForceCloseHandle1 (get_ttyp ()->from_slave, from_slave);
#endif
      if (get_ttyp ()->from_master)
        CloseHandle (get_ttyp ()->from_master);
      if (get_ttyp ()->to_master)
        CloseHandle (get_ttyp ()->to_master);

      fhandler_tty_common::close ();
[...]

That code closes the last  references to the master end pipes. Those
closed by fhandler_tty_common::close() are just references.

What happens is that although the code wants to make the slave end
encounter an EOF condition it rarely succeeds due to the particular order
of steps involved i.e.:

1) Signal the code for input event

2) Close one set of references to the master end pipes

3) Close the last set of references to the master end pipes

It is likely (and it happens most of the time with MC) that by the time
at which the slave ends tries to react on the input notification the
master end pipes are still not closed and it won't notice the EOF event.

The attached patch solves this problem by rearranging the code a bit. It
tries to be non-intrusive. I offer it for discussion and comments. I hope
that my description of the problem and the patch will help to resolve the
issue even if the patch will get rejected in favour of a better one.

Thanks!
-------------- next part --------------
Index: fhandler_tty.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler_tty.cc,v
retrieving revision 1.143
diff -u -p -r1.143 fhandler_tty.cc
--- fhandler_tty.cc	5 Aug 2005 16:11:21 -0000	1.143
+++ fhandler_tty.cc	17 Aug 2005 14:27:39 -0000
@@ -1196,6 +1196,10 @@ fhandler_tty_common::close ()
     termios_printf ("CloseHandle (input_mutex<%p>), %E", input_mutex);
   if (!ForceCloseHandle (output_mutex))
     termios_printf ("CloseHandle (output_mutex<%p>), %E", output_mutex);
+  if (!ForceCloseHandle1 (get_handle (), from_pty))
+    termios_printf ("CloseHandle (get_handle ()<%p>), %E", get_handle ());
+  if (!ForceCloseHandle1 (get_output_handle (), to_pty))
+    termios_printf ("CloseHandle (get_output_handle ()<%p>), %E", get_output_handle ());
 
   /* Send EOF to slaves if master side is closed */
   if (!get_ttyp ()->master_alive ())
@@ -1206,10 +1210,6 @@ fhandler_tty_common::close ()
 
   if (!ForceCloseHandle (input_available_event))
     termios_printf ("CloseHandle (input_available_event<%p>), %E", input_available_event);
-  if (!ForceCloseHandle1 (get_handle (), from_pty))
-    termios_printf ("CloseHandle (get_handle ()<%p>), %E", get_handle ());
-  if (!ForceCloseHandle1 (get_output_handle (), to_pty))
-    termios_printf ("CloseHandle (get_output_handle ()<%p>), %E", get_output_handle ());
 
   if (!hExeced)
     {
@@ -1226,7 +1226,6 @@ fhandler_pty_master::close ()
   while (accept_input () > 0)
     continue;
 #endif
-  fhandler_tty_common::close ();
 
   if (!get_ttyp ()->master_alive ())
     {
@@ -1241,9 +1240,14 @@ fhandler_pty_master::close ()
 	CloseHandle (get_ttyp ()->from_master);
       if (get_ttyp ()->to_master)
 	CloseHandle (get_ttyp ()->to_master);
+
+      fhandler_tty_common::close ();
+
       if (!hExeced)
 	get_ttyp ()->init ();
     }
+  else
+    fhandler_tty_common::close ();
 
   return 0;
 }


More information about the Cygwin-patches mailing list