X-Recipient: archive-cygwin AT delorie DOT com
DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id
	:list-unsubscribe:list-subscribe:list-archive:list-post
	:list-help:sender:date:from:to:subject:message-id:reply-to
	:references:mime-version:content-type:in-reply-to; q=dns; s=
	default; b=k9A5cEBL8IqnpE7eHTQgTRMA0+QoiExJl3a/RWsAlL91V0fB2RZzL
	rXwGWGEhY5Ee/W20tuBOS2aIxhmzGGv/P+pOnZRmgP9op8ip8XZk2jbGKte0C+WV
	IzABdiXg+bcOq17aMLtS+f715BWA66YFdqgVhCU+Fem9J+TW1A12xs=
DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id
	:list-unsubscribe:list-subscribe:list-archive:list-post
	:list-help:sender:date:from:to:subject:message-id:reply-to
	:references:mime-version:content-type:in-reply-to; s=default;
	 bh=XuzSC4vclHP/9RS3DFsegg5xZFg=; b=TJVw0mUoTougIQu1s3dftqF1tB97
	F149/vOPkNl+BeJyrIbS7oY5eMyqJklWIWq6rxolfB/7GAw8MSm3r3AT7HAU18DN
	rp6hWVPKlKPG5eUhO3hwf93alz3udYDwfh97lTgcLKrAT+hvlW2e6O7wqgFO3PKh
	If+V2rSU4s182zQ=
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
List-Id: <cygwin.cygwin.com>
List-Subscribe: <mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive: <http://sourceware.org/ml/cygwin/>
List-Post: <mailto:cygwin AT cygwin DOT com>
List-Help: <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Delivered-To: mailing list cygwin AT cygwin DOT com
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-4.9 required=5.0 tests=AWL,BAYES_00,UNSUBSCRIBE_BODY autolearn=no version=3.3.2
X-HELO: calimero.vinschen.de
Date: Wed, 4 Mar 2015 19:18:57 +0100
From: Corinna Vinschen <corinna-cygwin AT cygwin DOT com>
To: cygwin AT cygwin DOT com
Subject: Re: Cygwin hangs up if several keys are typed during outputting a lot of texts.
Message-ID: <20150304181857.GM3213@calimero.vinschen.de>
Reply-To: cygwin AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
References: <20150228144019 DOT 0e4cfdb3a26bfac361b538e2 AT nifty DOT ne DOT jp> <20150228140251 DOT GA11124 AT calimero DOT vinschen DOT de> <20150302204502 DOT 39b3e03ad5084b0b5add5d10 AT nifty DOT ne DOT jp> <20150302144426 DOT GK3213 AT calimero DOT vinschen DOT de> <20150304203407 DOT 14008531b0fb63ad5c19a33f AT nifty DOT ne DOT jp> <20150304121952 DOT GL3213 AT calimero DOT vinschen DOT de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha1;	protocol="application/pgp-signature"; boundary="vN1VrOpIkjMGbM/7"
Content-Disposition: inline
In-Reply-To: <20150304121952.GL3213@calimero.vinschen.de>
User-Agent: Mutt/1.5.23 (2014-03-12)

--vN1VrOpIkjMGbM/7
Content-Type: multipart/mixed; boundary="WG0/bXtUnGTsWt66"
Content-Disposition: inline


--WG0/bXtUnGTsWt66
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Takashi,

On Mar  4 13:19, Corinna Vinschen wrote:
> On Mar  4 20:34, Takashi Yano wrote:
> > On Mon, 2 Mar 2015 15:44:26 +0100
> > Corinna Vinschen <corinna-cygwin AT cygwin DOT com> wrote:
> > > > To check buffer space before WriteFile() is one idea,
> > > > but it is not smart, I suppose...
> > >=20
> > > I think that's not it.  [....blah...]
> > [...]
> > 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.
> >=20
> > Now, I suppose the cause has been clarified. So we have
> > to work out a solution. How can we resolve this problem?
>=20
> I'm not sure.  I still have to look deeper into that.  Locally
> I treid to woraround this problem by calling WriteFile overlapped
> without waiting for the result, but even that doesn't work.

Ok, the good news is, I have a potential workaround for this problem.
The bad news is, it might be just a bad hack.

The problem is apparently that the slave fills up the to_master buffer.
If the master doesn't constantly and fastly read the slave output, the
slave will hang a lot in the WriteFile call and the pipe buffer is most
of the time full.  When the master itself calls doecho, it has a rather
high probability that the slave is in a WriteFile call.  The master will
hang in acquire_output_mutex while the slave hangs in WriteFile.  Worse,
even when removing the acquire/release_output_mutex bracket, there's an
just as high probablity that the master will hang in the WriteFile call.

If the master process is not handling reading and writing from the pipe
in different threads, but just using a single-threaded select loop, the
fate of both processes is sealed.

The obvious culprit here is that the master side echos into the same
pipe write side the slave is writing to.  It occured to me that the
problem should disappear if we decouple echo output from slave output.
So the idea was that doecho should write into its own echo pipe and
fhandler_pty_master::process_slave_output as well as select's peek_pipe
both check for data in the echo pipe.

The result is the patch I attached to this mail.  I'm not sure it's the
most feasible way to solve this problem, but it works pretty nicely for
me, including pasting big chunks (I tried a clipboard with about 70K
of data) while yes(1) is running.

As a side effect, we can get rid of the acquire/release_output_mutex
bracketing which should speed up output slightly (but reading on the
master side is slightly slower now...)


Please have a look.


Thanks,
Corinna

--=20
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

--WG0/bXtUnGTsWt66
Content-Type: text/plain; charset=utf-8
Content-Disposition: attachment; filename="x.diff"
Content-Transfer-Encoding: quoted-printable

Index: fhandler.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/src/src/winsup/cygwin/fhandler.h,v
retrieving revision 1.513
diff -u -p -r1.513 fhandler.h
--- fhandler.h	24 Feb 2015 11:05:02 -0000	1.513
+++ fhandler.h	4 Mar 2015 18:11:38 -0000
@@ -414,6 +414,7 @@ public:
   virtual HANDLE& get_io_handle () { return io_handle; }
   virtual HANDLE& get_output_handle () { return io_handle; }
   virtual HANDLE get_stat_handle () { return pc.handle () ?: io_handle; }
+  virtual HANDLE get_echo_handle () const { return NULL; }
   virtual bool hit_eof () {return false;}
   virtual select_record *select_read (select_stuff *);
   virtual select_record *select_write (select_stuff *);
@@ -1569,11 +1570,13 @@ class fhandler_pty_master: public fhandl
   HANDLE master_ctl;		// Control socket for handle duplication
   cygthread *master_thread;	// Master control thread
   HANDLE from_master, to_master;
+  HANDLE echo_r, echo_w;
   DWORD dwProcessId;		// Owner of master handles
=20
 public:
   int need_nl;			// Next read should start with \n
=20
+  HANDLE get_echo_handle () const { return echo_r; }
   /* Constructor */
   fhandler_pty_master (int);
=20
Index: fhandler_tty.cc
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/src/src/winsup/cygwin/fhandler_tty.cc,v
retrieving revision 1.290
diff -u -p -r1.290 fhandler_tty.cc
--- fhandler_tty.cc	25 Feb 2015 16:46:57 -0000	1.290
+++ fhandler_tty.cc	4 Mar 2015 18:11:38 -0000
@@ -145,10 +145,8 @@ fhandler_pty_common::__release_output_mu
 void
 fhandler_pty_master::doecho (const void *str, DWORD len)
 {
-  acquire_output_mutex (INFINITE);
-  if (!WriteFile (to_master, str, len, &len, NULL))
-    termios_printf ("Write to %p failed, %E", to_master);
-  release_output_mutex ();
+  if (!WriteFile (echo_w, str, len, &len, NULL))
+    termios_printf ("Write to echo pipe failed, %E");
 }
=20
 int
@@ -221,6 +219,7 @@ fhandler_pty_master::process_slave_outpu
   size_t rlen;
   char outbuf[OUT_BUFFER_SIZE + 1];
   DWORD n;
+  DWORD echo_cnt;
   int column =3D 0;
   int rc =3D 0;
=20
@@ -257,9 +256,12 @@ fhandler_pty_master::process_slave_outpu
       if (rlen > sizeof outbuf)
 	rlen =3D sizeof outbuf;
=20
-      n =3D 0;
+      n =3D echo_cnt =3D 0;
       for (;;)
 	{
+	  /* Check echo pipe first. */
+	  if (::bytes_available (echo_cnt, echo_r) && echo_cnt > 0)
+	    break;
 	  if (!bytes_available (n))
 	    goto err;
 	  if (n)
@@ -287,7 +289,12 @@ fhandler_pty_master::process_slave_outpu
 	  flush_to_slave ();
 	}
=20
-      if (!ReadFile (get_handle (), outbuf, rlen, &n, NULL))
+      /* If echo pipe has data (something has been typed or pasted), prefer
+         it over slave output. */
+      if (echo_cnt > 0
+	  && ReadFile (echo_r, outbuf, MIN (rlen, echo_cnt), &n, NULL))
+	;
+      else if (!ReadFile (get_handle (), outbuf, rlen, &n, NULL))
 	{
 	  termios_printf ("ReadFile failed, %E");
 	  goto err;
@@ -653,7 +660,6 @@ fhandler_pty_slave::write (const void *p
=20
       while (tc ()->output_stopped)
 	cygwait (10);
-      acquire_output_mutex (INFINITE);
=20
       /* Previous write may have set write_error to !=3D 0.  Check it here.
 	 This is less than optimal, but the alternative slows down pty
@@ -663,12 +669,10 @@ fhandler_pty_slave::write (const void *p
 	  set_errno (get_ttyp ()->write_error);
 	  towrite =3D -1;
 	  get_ttyp ()->write_error =3D 0;
-	  release_output_mutex ();
 	  break;
 	}
=20
       BOOL res =3D WriteFile (get_output_handle (), buf, n, &n, NULL);
-      release_output_mutex ();
       if (!res)
 	{
 	  DWORD err =3D GetLastError ();
@@ -1228,7 +1232,7 @@ errout:
 fhandler_pty_master::fhandler_pty_master (int unit)
   : fhandler_pty_common (), pktmode (0), master_ctl (NULL),
     master_thread (NULL), from_master (NULL), to_master (NULL),
-    dwProcessId (0), need_nl (0)
+    echo_r (NULL), echo_w (NULL), dwProcessId (0), need_nl (0)
 {
   if (unit >=3D 0)
     dev ().parse (DEV_PTYM_MAJOR, unit);
@@ -1317,6 +1321,9 @@ fhandler_pty_master::close ()
   if (!ForceCloseHandle (to_master))
     termios_printf ("error closing from_master %p, %E", to_master);
   from_master =3D to_master =3D NULL;
+  ForceCloseHandle (echo_r);
+  ForceCloseHandle (echo_w);
+  echo_r =3D echo_w =3D NULL;
=20
   fhandler_pty_common::close ();
=20
@@ -1660,6 +1667,10 @@ fhandler_pty_master::setup ()
=20
   ProtectHandle1 (get_io_handle (), from_pty);
=20
+  __small_sprintf (pipename, "pty%d-echoloop", unit);
+  res =3D fhandler_pipe::create (&sec_none, &echo_r, &echo_w,
+			       fhandler_pty_common::pipesize, pipename, 0);
+
   /* Create security attribute.  Default permissions are 0620. */
   sd.malloc (sizeof (SECURITY_DESCRIPTOR));
   RtlCreateSecurityDescriptor (sd, SECURITY_DESCRIPTOR_REVISION);
@@ -1730,6 +1741,8 @@ err:
   close_maybe (input_mutex);
   close_maybe (from_master);
   close_maybe (to_master);
+  close_maybe (echo_r);
+  close_maybe (echo_w);
   close_maybe (master_ctl);
   termios_printf ("pty%d open failed - failed to create %s", unit, errstr);
   return false;
Index: select.cc
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvs/src/src/winsup/cygwin/select.cc,v
retrieving revision 1.223
diff -u -p -r1.223 select.cc
--- select.cc	11 Oct 2014 12:14:29 -0000	1.223
+++ select.cc	4 Mar 2015 18:11:38 -0000
@@ -626,6 +626,9 @@ peek_pipe (select_record *s, bool from_s
 	  goto out;
 	}
       int n =3D pipe_data_available (s->fd, fh, h, false);
+      /* On PTY masters, check if input from the echo pipe is available. */
+      if (n =3D=3D 0 && fh->get_echo_handle ())
+	n =3D pipe_data_available (s->fd, fh, fh->get_echo_handle (), false);
=20
       if (n < 0)
 	{

--WG0/bXtUnGTsWt66--

--vN1VrOpIkjMGbM/7
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJU90yRAAoJEPU2Bp2uRE+gLLMP/16P8uj5U1ptyyVvRtswDIHy
9K0S/lyBF9HmPF6g3cNboi/fPfWtwbhTeoEUkoovBRfWmNjUPMjG6tkel68mPU3u
7fiVaBHNr5d3syWWkXFePq1EGNGSvt5qo1t6rfcQedpwEVL25EoSIhjhMqMEwLgP
PbHJGWuhPVobWrK6oP3lnez2kd3lSsfEsJbY2UcB9u52KNZWqKw5YqqYeyOSmN30
ygwK2RsVgeey1hITRRMID43bKg4irlc9bP2Xx9dkNJKK1mT6GABHGvusoRaHv/E8
sPKPPlYDRRv8xJYn5WqR/hIzgowSIh4Qpb8VBxYJqyXKODp1nF9dGv3nBIrN3QBt
PanLepdtYcxITCvVS5Orj6jHyQJadmK5MJrdWxZL868icdRF2kh2X9ZIsTPy9c4G
GTqJgqpHV3NYB/CNFhDRwEVAmhC6QyhUoS6RwQR+DBzlSicXJmAyhWIEn1cIe8/Q
gybDE+E5i2Yzhq5NrmSJbDJalBoNQfRFp1slR2PauIpsVvJ6GOOueCTZ9b9HKRm6
XCVur44Zof3S5cphd9CtWVIRvKth5Y8nugscSa+i/dODQwsRyqJPxslTcySINDun
h6r5rVuamoKV+pjH7uezp7dHte85x3q8Xfvou2yyu5ocCkIF9ELa8au/KxjOhf9R
L6sZx9fWHwNhu92itksV
=f3Kv
-----END PGP SIGNATURE-----

--vN1VrOpIkjMGbM/7--