From 3bf693dde14b17e78ff99a0d04baf71841ae884c Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Thu, 5 Mar 2015 12:57:34 +0000 Subject: [PATCH] * fhandler.h (fhandler_base::get_echo_handle): New virtual method. (class fhandler_pty_master): Add echo_r and echo_w handles constituting read and write side of new echo pipe. * select.cc (peek_pipe): On pty masters, check additionally if input from the echo pipe is available. * fhandler_tty.cc (fhandler_pty_master::doecho): Drop output_mutex locking. Write output to echo pipe. (fhandler_pty_master::process_slave_output): Check if input is available in echo pipe and prefer to read from it, if so. (fhandler_pty_slave::write): Drop output_mutex locking. (fhandler_pty_master::fhandler_pty_master): Initialize echo pipe handles to NULL. (fhandler_pty_master::close): Close and NULL echo pipe handles. (fhandler_pty_master::setup): Create echo pipe, close in case of error. --- winsup/cygwin/ChangeLog | 17 ++++++++++++++ winsup/cygwin/fhandler.h | 3 +++ winsup/cygwin/fhandler_tty.cc | 43 +++++++++++++++++++++++++++-------- winsup/cygwin/release/1.7.36 | 14 ++++++++++++ winsup/cygwin/select.cc | 5 +++- 5 files changed, 71 insertions(+), 11 deletions(-) create mode 100644 winsup/cygwin/release/1.7.36 diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 9f3130ba3..92b0cf82c 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,20 @@ +2015-03-05 Corinna Vinschen + + * fhandler.h (fhandler_base::get_echo_handle): New virtual method. + (class fhandler_pty_master): Add echo_r and echo_w handles constituting + read and write side of new echo pipe. + * select.cc (peek_pipe): On pty masters, check additionally if input + from the echo pipe is available. + * fhandler_tty.cc (fhandler_pty_master::doecho): Drop output_mutex + locking. Write output to echo pipe. + (fhandler_pty_master::process_slave_output): Check if input is available + in echo pipe and prefer to read from it, if so. + (fhandler_pty_slave::write): Drop output_mutex locking. + (fhandler_pty_master::fhandler_pty_master): Initialize echo pipe + handles to NULL. + (fhandler_pty_master::close): Close and NULL echo pipe handles. + (fhandler_pty_master::setup): Create echo pipe, close in case of error. + 2015-03-04 Corinna Vinschen * include/cygwin/version.h (CYGWIN_VERSION_DLL_MINOR): Bump to 36. diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h index 10eacd106..4ec7d02fa 100644 --- a/winsup/cygwin/fhandler.h +++ b/winsup/cygwin/fhandler.h @@ -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 fhandler_pty_common 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); diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc index 9d793281a..39fe6dffc 100644 --- a/winsup/cygwin/fhandler_tty.cc +++ b/winsup/cygwin/fhandler_tty.cc @@ -145,10 +145,8 @@ fhandler_pty_common::__release_output_mutex (const char *fn, int ln) 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_output (char *buf, size_t len, int pktmode_on 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_output (char *buf, size_t len, int pktmode_on 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,17 @@ fhandler_pty_master::process_slave_output (char *buf, size_t len, int pktmode_on 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) + { + if (!ReadFile (echo_r, outbuf, rlen, &n, NULL)) + { + termios_printf ("ReadFile on echo pipe failed, %E"); + goto err; + } + } + else if (!ReadFile (get_handle (), outbuf, rlen, &n, NULL)) { termios_printf ("ReadFile failed, %E"); goto err; @@ -653,7 +665,6 @@ fhandler_pty_slave::write (const void *ptr, size_t len) 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 +674,10 @@ fhandler_pty_slave::write (const void *ptr, size_t len) 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 +1237,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 +1326,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 +1672,15 @@ 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); + if (res) + { + errstr = "echo pipe"; + goto err; + } + /* Create security attribute. Default permissions are 0620. */ sd.malloc (sizeof (SECURITY_DESCRIPTOR)); RtlCreateSecurityDescriptor (sd, SECURITY_DESCRIPTOR_REVISION); @@ -1730,6 +1751,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; diff --git a/winsup/cygwin/release/1.7.36 b/winsup/cygwin/release/1.7.36 new file mode 100644 index 000000000..308a77225 --- /dev/null +++ b/winsup/cygwin/release/1.7.36 @@ -0,0 +1,14 @@ +What's new: +----------- + + +What changed: +------------- + + +Bug Fixes +--------- + +- Fix potential hang in pseudo ttys when generating ECHO output while the slave + is flooding the pty with output. + Addresses: https://cygwin.com/ml/cygwin/2015-03/msg00019.html diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc index c72cd0c76..17a32a367 100644 --- a/winsup/cygwin/select.cc +++ b/winsup/cygwin/select.cc @@ -1,7 +1,7 @@ /* select.cc Copyright 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, - 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014 Red Hat, Inc. + 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Red Hat, Inc. This file is part of Cygwin. @@ -626,6 +626,9 @@ peek_pipe (select_record *s, bool from_select) 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) {