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-cyg...@cygwin.com> wrote: > > > > To check buffer space before WriteFile() is one idea, > > > > but it is not smart, I suppose... > > > > > > 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. > > > > Now, I suppose the cause has been clarified. So we have > > to work out a solution. How can we resolve this problem? > > 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 -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat
Index: fhandler.h
===================================================================
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
public:
int need_nl; // Next read should start with \n
+ HANDLE get_echo_handle () const { return echo_r; }
/* Constructor */
fhandler_pty_master (int);
Index: 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
--- 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");
}
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 = 0;
int rc = 0;
@@ -257,9 +256,12 @@ fhandler_pty_master::process_slave_outpu
if (rlen > sizeof outbuf)
rlen = sizeof outbuf;
- n = 0;
+ n = echo_cnt = 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 ();
}
- 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
while (tc ()->output_stopped)
cygwait (10);
- acquire_output_mutex (INFINITE);
/* Previous write may have set write_error to != 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 = -1;
get_ttyp ()->write_error = 0;
- release_output_mutex ();
break;
}
BOOL res = WriteFile (get_output_handle (), buf, n, &n, NULL);
- release_output_mutex ();
if (!res)
{
DWORD err = 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 >= 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 = to_master = NULL;
+ ForceCloseHandle (echo_r);
+ ForceCloseHandle (echo_w);
+ echo_r = echo_w = NULL;
fhandler_pty_common::close ();
@@ -1660,6 +1667,10 @@ fhandler_pty_master::setup ()
ProtectHandle1 (get_io_handle (), from_pty);
+ __small_sprintf (pipename, "pty%d-echoloop", unit);
+ res = 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
===================================================================
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 = pipe_data_available (s->fd, fh, h, false);
+ /* On PTY masters, check if input from the echo pipe is available. */
+ if (n == 0 && fh->get_echo_handle ())
+ n = pipe_data_available (s->fd, fh, fh->get_echo_handle (), false);
if (n < 0)
{
pgpXF5mfMiPb2.pgp
Description: PGP signature
