Re: Cygwin hangs up if several keys are typed during outputting a lot of texts.

[Takashi Yano <takashi.yano@nifty.ne.jp>]

[-- Attachment #1: Type: text/plain, Size: 1617 bytes --]

On Mon, 2 Mar 2015 15:44:26 +0100
Corinna Vinschen <corinna-cygwin@cygwin.com> wrote:

> > 1) Buffer of named pipe gets full-filled by a lot of data written
> >   by slave side.
> > 2) WriteFile() in fhandler_pty_master::doecho(), which is called
> >   from master side by key input, is blocked because the buffer
> >   is full.
> > 3) If a handling to read from the pipe is in the same thread as
> >   key input, the thread falls into deadlock.
> > 
> > To check buffer space before WriteFile() is one idea,
> > but it is not smart, I suppose...
> 
> I think that's not it.  For testing I added code to convert the
> WriteFile calls in fhandler_pty_slave::write, fhandler_pty_master::doecho
> and fhandler_pty_slave::write to overlapped I/O and made sure to wait
> for the result after the output mutex has been released.
> 
> This change has no effect at all.  Looks like this needs some more
> digging.

Hmm.

To confirm my assumption, I have modified the TTY code
for testing. The modification is shown in an attached
diff-file against current CVS version. (Note that this
patch is not for fixing the problem. It is for just a
testing.)

With this modification, half of buffer is kept empty
for fhandler_pty_master::doecho(). I assumed the blocking
could be avoided because buffer is always free.

As a result of the test, it has become clear that this
modification conceals the problem. This means the problem
is closely related to occupancy of buffer.

Now, I suppose the cause has been clarified. So we have
to work out a solution. How can we resolve this problem?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: cygwin-test.patch.20150304 --]
[-- Type: application/octet-stream, Size: 7665 bytes --]

Index: src/winsup/cygwin/fhandler.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler.h,v
retrieving revision 1.513
diff -u -p -r1.513 fhandler.h
--- src/winsup/cygwin/fhandler.h	24 Feb 2015 11:05:02 -0000	1.513
+++ src/winsup/cygwin/fhandler.h	4 Mar 2015 11:01:27 -0000
@@ -1512,6 +1512,7 @@ class fhandler_pty_common: public fhandl
 class fhandler_pty_slave: public fhandler_pty_common
 {
   HANDLE inuse;			// used to indicate that a tty is in use
+  HANDLE from_slave, to_slave;	// for testing
 
   /* Helper functions for fchmod and fchown. */
   bool fch_open_handles (bool chown);
Index: src/winsup/cygwin/fhandler_tty.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler_tty.cc,v
retrieving revision 1.290
diff -u -p -r1.290 fhandler_tty.cc
--- src/winsup/cygwin/fhandler_tty.cc	25 Feb 2015 16:46:57 -0000	1.290
+++ src/winsup/cygwin/fhandler_tty.cc	4 Mar 2015 11:01:27 -0000
@@ -42,6 +42,8 @@ struct pipe_request {
 struct pipe_reply {
   HANDLE from_master;
   HANDLE to_master;
+  HANDLE from_slave;
+  HANDLE to_slave;
   DWORD error;
 };
 
@@ -378,7 +380,8 @@ out:
 /* pty slave stuff */
 
 fhandler_pty_slave::fhandler_pty_slave (int unit)
-  : fhandler_pty_common (), inuse (NULL)
+  : fhandler_pty_common (), inuse (NULL),
+    from_slave (NULL), to_slave (NULL)
 {
   if (unit >= 0)
     dev ().parse (DEV_PTYS_MAJOR, unit);
@@ -388,10 +391,12 @@ int
 fhandler_pty_slave::open (int flags, mode_t)
 {
   HANDLE pty_owner, from_master_local, to_master_local;
+  HANDLE from_slave_local, to_slave_local;
   HANDLE *handles[] =
   {
     &from_master_local, &input_available_event, &input_mutex, &inuse,
     &output_mutex, &to_master_local, &pty_owner,
+    &from_slave_local, &to_slave_local,
     NULL
   };
 
@@ -500,6 +505,22 @@ fhandler_pty_slave::open (int flags, mod
 	  errmsg = "can't duplicate output, %E";
 	  goto err;
 	}
+      if (!DuplicateHandle (pty_owner, get_ttyp ()->from_slave (),
+			    GetCurrentProcess (), &from_slave_local, 0, TRUE,
+			    DUPLICATE_SAME_ACCESS))
+	{
+	  termios_printf ("can't duplicate master input from %u/%p, %E",
+			  get_ttyp ()->master_pid, get_ttyp ()->from_slave ());
+	  __seterrno ();
+	  goto err_no_msg;
+	}
+      if (!DuplicateHandle (pty_owner, get_ttyp ()->to_slave (),
+			  GetCurrentProcess (), &to_slave_local, 0, TRUE,
+			  DUPLICATE_SAME_ACCESS))
+	{
+	  errmsg = "can't duplicate master output, %E";
+	  goto err;
+	}
       if (pty_owner != GetCurrentProcess ())
 	CloseHandle (pty_owner);
     }
@@ -520,23 +541,39 @@ fhandler_pty_slave::open (int flags, mod
 	}
       from_master_local = repl.from_master;
       to_master_local = repl.to_master;
+      from_slave_local = repl.from_slave;
+      to_slave_local = repl.to_slave;
       if (!from_master_local || !to_master_local)
 	{
 	  SetLastError (repl.error);
 	  errmsg = "error duplicating pipes, %E";
 	  goto err;
 	}
+      if (!from_slave_local || !to_slave_local)
+	{
+	  SetLastError (repl.error);
+	  errmsg = "error duplicating master pipes, %E";
+	  goto err;
+	}
     }
   VerifyHandle (from_master_local);
   VerifyHandle (to_master_local);
+  VerifyHandle (from_slave_local);
+  VerifyHandle (to_slave_local);
 
   termios_printf ("duplicated from_master %p->%p from pty_owner",
 		  get_ttyp ()->from_master (), from_master_local);
   termios_printf ("duplicated to_master %p->%p from pty_owner",
 		  get_ttyp ()->to_master (), to_master_local);
+  termios_printf ("duplicated from_slave %p->%p from pty_owner",
+		  get_ttyp ()->from_slave (), from_slave_local);
+  termios_printf ("duplicated to_slave %p->%p from pty_owner",
+		  get_ttyp ()->to_slave (), to_slave_local);
 
   set_io_handle (from_master_local);
   set_output_handle (to_master_local);
+  from_slave = from_slave_local;
+  to_slave = to_slave_local;
 
   fhandler_console::need_invisible ();
   set_open_status ();
@@ -587,6 +624,10 @@ fhandler_pty_slave::close ()
     termios_printf ("CloseHandle (input_available_event<%p>), %E", input_available_event);
   if ((unsigned) myself->ctty == FHDEV (DEV_PTYS_MAJOR, get_minor ()))
     fhandler_console::free_console ();	/* assumes that we are the last pty closer */
+  if (!ForceCloseHandle (from_slave))
+    termios_printf ("error closing from_slave %p, %E", from_slave);
+  if (!ForceCloseHandle (to_slave))
+    termios_printf ("error closing to_slave %p, %E", to_slave);
   return fhandler_pty_common::close ();
 }
 
@@ -653,6 +694,28 @@ fhandler_pty_slave::write (const void *p
 
       while (tc ()->output_stopped)
 	cygwait (10);
+
+      /* for testing */
+      DWORD buflen;
+      if (!GetNamedPipeInfo (get_output_handle (), NULL, &buflen, NULL, NULL))
+	{
+	  termios_printf ("GetNamedPipeInfo(%p) failed, %E", get_output_handle ());
+	  buflen = fhandler_pty_common::pipesize;
+	}
+      DWORD occupied;
+      if (!PeekNamedPipe (from_slave, NULL, 0, NULL, &occupied, NULL))
+	{
+	  termios_printf ("PeekNamedPipe(%p) failed, %E", from_slave);
+	}
+      else
+	{
+	  while (n + occupied > buflen / 2)
+	    {
+	      cygwait (10);
+	      PeekNamedPipe (from_slave, NULL, 0, NULL, &occupied, NULL);
+	    }
+	}
+
       acquire_output_mutex (INFINITE);
 
       /* Previous write may have set write_error to != 0.  Check it here.
@@ -1597,6 +1660,20 @@ fhandler_pty_master::pty_master_thread (
 	      termios_printf ("DuplicateHandle (to_master), %E");
 	      goto reply;
 	    }
+	  if (!DuplicateHandle (GetCurrentProcess (), get_io_handle (),
+			       client, &repl.from_slave,
+			       0, TRUE, DUPLICATE_SAME_ACCESS))
+	    {
+	      termios_printf ("DuplicateHandle (from_slave), %E");
+	      goto reply;
+	    }
+	  if (!DuplicateHandle (GetCurrentProcess (), get_output_handle (),
+				client, &repl.to_slave,
+				0, TRUE, DUPLICATE_SAME_ACCESS))
+	    {
+	      termios_printf ("DuplicateHandle (to_slave), %E");
+	      goto reply;
+	    }
 	}
 reply:
       repl.error = GetLastError ();
@@ -1711,6 +1788,8 @@ fhandler_pty_master::setup ()
 
   t.set_from_master (from_master);
   t.set_to_master (to_master);
+  t.set_from_slave (get_io_handle ());
+  t.set_to_slave (get_output_handle ());
   t.winsize.ws_col = 80;
   t.winsize.ws_row = 25;
   t.master_pid = myself->pid;
Index: src/winsup/cygwin/tty.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/tty.h,v
retrieving revision 1.37
diff -u -p -r1.37 tty.h
--- src/winsup/cygwin/tty.h	23 Apr 2013 09:44:34 -0000	1.37
+++ src/winsup/cygwin/tty.h	4 Mar 2015 11:01:27 -0000
@@ -104,15 +104,29 @@ private:
     HANDLE _to_master;
     LARGE_INTEGER _tm_dummy;
   };
+  union {
+    HANDLE _from_slave;
+    LARGE_INTEGER _fs_dummy;
+  };
+  union {    
+    HANDLE _to_slave;
+    LARGE_INTEGER _ts_dummy;
+  };
 public:
   HANDLE from_master() const { return _from_master; }
   HANDLE to_master() const { return _to_master; }
+  HANDLE from_slave() const { return _from_slave; }
+  HANDLE to_slave() const { return _to_slave; }
 #ifdef __x86_64__
   void set_from_master (HANDLE h) { _from_master = h; }
   void set_to_master (HANDLE h) { _to_master = h; }
+  void set_from_slave (HANDLE h) { _from_slave = h; }
+  void set_to_slave (HANDLE h) { _to_slave = h; }
 #else
   void set_from_master (HANDLE h) { _fm_dummy.HighPart = 0; _from_master = h; }
   void set_to_master (HANDLE h) { _tm_dummy.HighPart = 0; _to_master = h; }
+  void set_from_slave (HANDLE h) { _fs_dummy.HighPart = 0; _from_slave = h; }
+  void set_to_slave (HANDLE h) { _ts_dummy.HighPart = 0; _to_slave = h; }
 #endif
 
   int read_retval;

[-- Attachment #3: Type: text/plain, Size: 218 bytes --]

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple