public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
* [newlib-cygwin] Cygwin: console: Revise the code to switch xterm mode.
@ 2021-01-18 10:23 Corinna Vinschen
  0 siblings, 0 replies; only message in thread
From: Corinna Vinschen @ 2021-01-18 10:23 UTC (permalink / raw)
  To: cygwin-cvs

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=974e6d76d85a284add338d95bea9703e16a693d6

commit 974e6d76d85a284add338d95bea9703e16a693d6
Author: Takashi Yano via Cygwin-patches <cygwin-patches@cygwin.com>
Date:   Fri Jan 15 17:32:10 2021 +0900

    Cygwin: console: Revise the code to switch xterm mode.
    
    - If application changes the console mode, mode management introduced
      by commit 10d8c278 will be corrupted. For example, stdout of jansi
      v2.0.1 or later is piped to less, jansi resets the xterm mode flag
      ENABLE_VIRTUAL_TERMINA_PROCESSING when jansi is terminated. This
      causes garbled output in less because less needs this flag enabled.
      This patch fixes the issue.

Diff:
---
 winsup/cygwin/fhandler.h          |  18 ++++--
 winsup/cygwin/fhandler_console.cc | 121 +++++++++++++++++++++++++-------------
 winsup/cygwin/select.cc           |  15 +----
 winsup/cygwin/spawn.cc            |  35 ++++++++++-
 4 files changed, 127 insertions(+), 62 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index fe76c0781..45ac17af2 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2048,8 +2048,6 @@ class dev_console
   bool raw_win32_keyboard_mode;
   char cons_rabuf[40];  // cannot get longer than char buf[40] in char_command
   char *cons_rapoi;
-  LONG xterm_mode_input;
-  LONG xterm_mode_output;
   bool cursor_key_app_mode;
 
   inline UINT get_console_cp ();
@@ -2086,11 +2084,19 @@ public:
     input_signalled = 2,
     input_winch = 3
   };
+  struct handle_set_t
+  {
+    HANDLE input_handle;
+    HANDLE output_handle;
+    HANDLE input_mutex;
+    HANDLE output_mutex;
+  };
 private:
   static const unsigned MAX_WRITE_CHARS;
   static console_state *shared_console_info;
   static bool invisible_console;
   HANDLE input_mutex, output_mutex;
+  handle_set_t handle_set;
 
   /* Used when we encounter a truncated multi-byte sequence.  The
      lead bytes are stored here and revisited in the next write call. */
@@ -2212,8 +2218,12 @@ private:
   size_t &raixput ();
   size_t &rabuflen ();
 
-  void request_xterm_mode_input (bool);
-  void request_xterm_mode_output (bool);
+  const handle_set_t *get_handle_set (void) {return &handle_set;}
+  void get_duplicated_handle_set (handle_set_t *p);
+  static void close_handle_set (handle_set_t *p);
+
+  static void request_xterm_mode_input (bool, const handle_set_t *p);
+  static void request_xterm_mode_output (bool, const handle_set_t *p);
 
   friend tty_min * tty_list::get_cttyp ();
 };
diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 41cac37e6..a4c054e24 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -187,11 +187,7 @@ fhandler_console::set_unit ()
 	  tty_min_state.setntty (DEV_CONS_MAJOR, console_unit (me));
       devset = (fh_devices) shared_console_info->tty_min_state.getntty ();
       if (created)
-	{
-	  con.owner = myself->pid;
-	  con.xterm_mode_input = 0;
-	  con.xterm_mode_output = 0;
-	}
+	con.owner = myself->pid;
     }
   if (!created && shared_console_info)
     {
@@ -279,60 +275,65 @@ fhandler_console::rabuflen ()
   return con_ra.rabuflen;
 }
 
+/* The function request_xterm_mode_{in,out}put() should be static so that
+   they can be called even after the fhandler_console instance is deleted. */
 void
-fhandler_console::request_xterm_mode_input (bool req)
+fhandler_console::request_xterm_mode_input (bool req, const handle_set_t *p)
 {
   if (con_is_legacy)
     return;
+  WaitForSingleObject (p->input_mutex, INFINITE);
+  DWORD dwMode;
+  GetConsoleMode (p->input_handle, &dwMode);
   if (req)
     {
-      if (InterlockedExchange (&con.xterm_mode_input, 1) == 0)
+      if (!(dwMode & ENABLE_VIRTUAL_TERMINAL_INPUT))
 	{
-	  DWORD dwMode;
-	  GetConsoleMode (get_handle (), &dwMode);
 	  dwMode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
-	  SetConsoleMode (get_handle (), dwMode);
+	  SetConsoleMode (p->input_handle, dwMode);
 	  if (con.cursor_key_app_mode) /* Restore DECCKM */
-	    WriteConsoleW (get_output_handle (), L"\033[?1h", 5, NULL, 0);
+	    {
+	      request_xterm_mode_output (true, p);
+	      WriteConsoleW (p->output_handle, L"\033[?1h", 5, NULL, 0);
+	    }
 	}
     }
   else
     {
-      if (InterlockedExchange (&con.xterm_mode_input, 0) == 1)
+      if (dwMode & ENABLE_VIRTUAL_TERMINAL_INPUT)
 	{
-	  DWORD dwMode;
-	  GetConsoleMode (get_handle (), &dwMode);
 	  dwMode &= ~ENABLE_VIRTUAL_TERMINAL_INPUT;
-	  SetConsoleMode (get_handle (), dwMode);
+	  SetConsoleMode (p->input_handle, dwMode);
 	}
     }
+  ReleaseMutex (p->input_mutex);
 }
 
 void
-fhandler_console::request_xterm_mode_output (bool req)
+fhandler_console::request_xterm_mode_output (bool req, const handle_set_t *p)
 {
   if (con_is_legacy)
     return;
+  WaitForSingleObject (p->output_mutex, INFINITE);
+  DWORD dwMode;
+  GetConsoleMode (p->output_handle, &dwMode);
   if (req)
     {
-      if (InterlockedExchange (&con.xterm_mode_output, 1) == 0)
+      if (!(dwMode & ENABLE_VIRTUAL_TERMINAL_PROCESSING))
 	{
-	  DWORD dwMode;
-	  GetConsoleMode (get_output_handle (), &dwMode);
 	  dwMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
-	  SetConsoleMode (get_output_handle (), dwMode);
+	  SetConsoleMode (p->output_handle, dwMode);
 	}
     }
   else
     {
-      if (InterlockedExchange (&con.xterm_mode_output, 0) == 1)
+      if (dwMode & ENABLE_VIRTUAL_TERMINAL_PROCESSING)
 	{
-	  DWORD dwMode;
-	  GetConsoleMode (get_output_handle (), &dwMode);
 	  dwMode &= ~ENABLE_VIRTUAL_TERMINAL_PROCESSING;
-	  SetConsoleMode (get_output_handle (), dwMode);
+	  SetConsoleMode (p->output_handle, dwMode);
 	}
     }
+  ReleaseMutex (p->output_mutex);
 }
 
 /* Return the tty structure associated with a given tty number.  If the
@@ -440,8 +441,8 @@ fhandler_console::fix_tab_position (void)
 {
   /* Re-setting ENABLE_VIRTUAL_TERMINAL_PROCESSING
      fixes the tab position. */
-  request_xterm_mode_output (false);
-  request_xterm_mode_output (true);
+  request_xterm_mode_output (false, &handle_set);
+  request_xterm_mode_output (true, &handle_set);
 }
 
 bool
@@ -506,7 +507,7 @@ fhandler_console::read (void *pv, size_t& buflen)
 
   /* if system has 24 bit color capability, use xterm compatible mode. */
   if (wincap.has_con_24bit_colors ())
-    request_xterm_mode_input (true);
+    request_xterm_mode_input (true, &handle_set);
 
   while (!input_ready && !get_cons_readahead_valid ())
     {
@@ -514,8 +515,6 @@ fhandler_console::read (void *pv, size_t& buflen)
       if ((bgres = bg_check (SIGTTIN)) <= bg_eof)
 	{
 	  buflen = bgres;
-	  if (wincap.has_con_24bit_colors ())
-	    request_xterm_mode_input (false);
 	  return;
 	}
 
@@ -533,8 +532,6 @@ fhandler_console::read (void *pv, size_t& buflen)
 	case WAIT_TIMEOUT:
 	  set_sig_errno (EAGAIN);
 	  buflen = (size_t) -1;
-	  if (wincap.has_con_24bit_colors ())
-	    request_xterm_mode_input (false);
 	  return;
 	default:
 	  goto err;
@@ -582,22 +579,16 @@ fhandler_console::read (void *pv, size_t& buflen)
 #undef buf
 
   buflen = copied_chars;
-  if (wincap.has_con_24bit_colors ())
-    request_xterm_mode_input (false);
   return;
 
 err:
   __seterrno ();
   buflen = (size_t) -1;
-  if (wincap.has_con_24bit_colors ())
-    request_xterm_mode_input (false);
   return;
 
 sig_exit:
   set_sig_errno (EINTR);
   buflen = (size_t) -1;
-  if (wincap.has_con_24bit_colors ())
-    request_xterm_mode_input (false);
 }
 
 fhandler_console::input_states
@@ -1106,6 +1097,7 @@ fhandler_console::open (int flags, mode_t)
       return 0;
     }
   set_handle (h);
+  handle_set.input_handle = h;
 
   h = CreateFileW (L"CONOUT$", GENERIC_READ | GENERIC_WRITE,
 		  FILE_SHARE_READ | FILE_SHARE_WRITE, &sec_none,
@@ -1117,8 +1109,11 @@ fhandler_console::open (int flags, mode_t)
       return 0;
     }
   set_output_handle (h);
+  handle_set.output_handle = h;
 
   setup_io_mutex ();
+  handle_set.input_mutex = input_mutex;
+  handle_set.output_mutex = output_mutex;
 
   if (con.fillin (get_output_handle ()))
     {
@@ -1191,8 +1186,10 @@ fhandler_console::close ()
 			      &obi, sizeof obi, NULL);
       if ((NT_SUCCESS (status) && obi.HandleCount == 1)
 	  || myself->pid == con.owner)
-	request_xterm_mode_output (false);
-      request_xterm_mode_input (false);
+	{
+	  request_xterm_mode_output (false, &handle_set);
+	  request_xterm_mode_input (false, &handle_set);
+	}
     }
 
   release_output_mutex ();
@@ -1354,10 +1351,14 @@ fhandler_console::output_tcsetattr (int, struct termios const *t)
   /* All the output bits we can ignore */
 
   acquire_output_mutex (INFINITE);
-  if (wincap.has_con_24bit_colors ())
-    request_xterm_mode_output (false);
   DWORD flags = ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT;
 
+  DWORD oflags;
+  GetConsoleMode (get_output_handle (), &oflags);
+  if (wincap.has_con_24bit_colors () && !con_is_legacy
+      && (oflags & ENABLE_VIRTUAL_TERMINAL_PROCESSING))
+    flags |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
+
   int res = SetConsoleMode (get_output_handle (), flags) ? 0 : -1;
   if (res)
     __seterrno_from_win_error (GetLastError ());
@@ -1419,6 +1420,10 @@ fhandler_console::input_tcsetattr (int, struct termios const *t)
     ((wincap.has_con_24bit_colors () && !con_is_legacy) ?
      0 : ENABLE_MOUSE_INPUT);
 
+  if (wincap.has_con_24bit_colors () && !con_is_legacy
+      && (oflags & ENABLE_VIRTUAL_TERMINAL_INPUT))
+    flags |= ENABLE_VIRTUAL_TERMINAL_INPUT;
+
   int res;
   if (flags == oflags)
     res = 0;
@@ -2973,7 +2978,7 @@ fhandler_console::write (const void *vsrc, size_t len)
 
   /* If system has 24 bit color capability, use xterm compatible mode. */
   if (wincap.has_con_24bit_colors ())
-    request_xterm_mode_output (true);
+    request_xterm_mode_output (true, &handle_set);
   if (wincap.has_con_24bit_colors () && !con_is_legacy)
     {
       DWORD dwMode;
@@ -3657,3 +3662,35 @@ fhandler_console::__release_output_mutex (const char *fn, int ln)
   strace.prntf (_STRACE_TERMIOS, fn, "(%d): release output_mutex", ln);
 #endif
 }
+
+void
+fhandler_console::get_duplicated_handle_set (handle_set_t *p)
+{
+  DuplicateHandle (GetCurrentProcess (), get_handle (),
+		   GetCurrentProcess (), &p->input_handle,
+		   0, FALSE, DUPLICATE_SAME_ACCESS);
+  DuplicateHandle (GetCurrentProcess (), get_output_handle (),
+		   GetCurrentProcess (), &p->output_handle,
+		   0, FALSE, DUPLICATE_SAME_ACCESS);
+  DuplicateHandle (GetCurrentProcess (), input_mutex,
+		   GetCurrentProcess (), &p->input_mutex,
+		   0, FALSE, DUPLICATE_SAME_ACCESS);
+  DuplicateHandle (GetCurrentProcess (), output_mutex,
+		   GetCurrentProcess (), &p->output_mutex,
+		   0, FALSE, DUPLICATE_SAME_ACCESS);
+}
+
+/* The function close_handle_set() should be static so that they can
+   be called even after the fhandler_console instance is deleted. */
+void
+fhandler_console::close_handle_set (handle_set_t *p)
+{
+  CloseHandle (p->input_handle);
+  p->input_handle = NULL;
+  CloseHandle (p->output_handle);
+  p->output_handle = NULL;
+  CloseHandle (p->input_mutex);
+  p->input_mutex = NULL;
+  CloseHandle (p->output_mutex);
+  p->output_mutex = NULL;
+}
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 501714fa7..dcb9b2d6e 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -1087,28 +1087,15 @@ verify_console (select_record *me, fd_set *rfds, fd_set *wfds,
   return peek_console (me, true);
 }
 
-static void console_cleanup (select_record *, select_stuff *);
-
 static int
 console_startup (select_record *me, select_stuff *stuff)
 {
   fhandler_console *fh = (fhandler_console *) me->fh;
   if (wincap.has_con_24bit_colors ())
-    {
-      fh->request_xterm_mode_input (true);
-      me->cleanup = console_cleanup;
-    }
+    fhandler_console::request_xterm_mode_input (true, fh->get_handle_set ());
   return 1;
 }
 
-static void
-console_cleanup (select_record *me, select_stuff *stuff)
-{
-  fhandler_console *fh = (fhandler_console *) me->fh;
-  if (wincap.has_con_24bit_colors ())
-    fh->request_xterm_mode_input (false);
-}
-
 select_record *
 fhandler_console::select_read (select_stuff *ss)
 {
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 5057af932..94909df4c 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -606,6 +606,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	sa = &sec_none_nih;
 
       fhandler_pty_slave *ptys_primary = NULL;
+      fhandler_console *cons_native = NULL;
       for (int i = 0; i < 3; i ++)
 	{
 	  const int chk_order[] = {1, 0, 2};
@@ -621,10 +622,23 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	    {
 	      fhandler_console *cons = (fhandler_console *) fh;
 	      if (wincap.has_con_24bit_colors () && !iscygwin ())
-		if (fd == 1 || fd == 2)
-		  cons->request_xterm_mode_output (false);
+		{
+		  if (cons_native == NULL)
+		    cons_native = cons;
+		  if (fd == 0)
+		    fhandler_console::request_xterm_mode_input (false,
+						cons->get_handle_set ());
+		  else if (fd == 1 || fd == 2)
+		    fhandler_console::request_xterm_mode_output (false,
+						 cons->get_handle_set ());
+		}
 	    }
 	}
+      struct fhandler_console::handle_set_t cons_handle_set = { 0, };
+      if (cons_native)
+	/* Console handles will be closed by close_all_handle(),
+	   therefore, duplicate them here */
+	cons_native->get_duplicated_handle_set (&cons_handle_set);
 
       if (!iscygwin ())
 	{
@@ -942,6 +956,15 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	      WaitForSingleObject (pi.hProcess, INFINITE);
 	      ptys_primary->close_pseudoconsole ();
 	    }
+	  else if (cons_native)
+	    {
+	      WaitForSingleObject (pi.hProcess, INFINITE);
+	      fhandler_console::request_xterm_mode_output (true,
+							   &cons_handle_set);
+	      fhandler_console::request_xterm_mode_input (true,
+							  &cons_handle_set);
+	      fhandler_console::close_handle_set (&cons_handle_set);
+	    }
 	  myself.exit (EXITCODE_NOSET);
 	  break;
 	case _P_WAIT:
@@ -951,6 +974,14 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	    res = -1;
 	  if (enable_pcon)
 	    ptys_primary->close_pseudoconsole ();
+	  else if (cons_native)
+	    {
+	      fhandler_console::request_xterm_mode_output (true,
+							   &cons_handle_set);
+	      fhandler_console::request_xterm_mode_input (true,
+							  &cons_handle_set);
+	      fhandler_console::close_handle_set (&cons_handle_set);
+	    }
 	  break;
 	case _P_DETACH:
 	  res = 0;	/* Lost all memory of this child. */


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-01-18 10:23 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-18 10:23 [newlib-cygwin] Cygwin: console: Revise the code to switch xterm mode Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).