[PATCH v3 0/2] Make terminal read() thread-safe.

[PATCH v3 0/2] Make terminal read() thread-safe.

[Takashi Yano]

Currently read() for console and pty slave are somehow
not thread-safe. These patches fix the issue.

Takashi Yano (2):
  Cygwin: console: Make read() thread-safe.
  Cygwin: pty: Make slave read() thread-safe.

 winsup/cygwin/fhandler.h          | 10 ++++++++++
 winsup/cygwin/fhandler_console.cc | 11 ++++++-----
 winsup/cygwin/fhandler_termios.cc |  2 ++
 winsup/cygwin/fhandler_tty.cc     |  6 ++++++
 4 files changed, 24 insertions(+), 5 deletions(-)

-- 
2.30.0

[PATCH v3 1/2] Cygwin: console: Make read() thread-safe.

[Takashi Yano]

- Currently read() is somehow not thread-safe. This patch fixes
  the issue.
---
 winsup/cygwin/fhandler.h          | 10 ++++++++++
 winsup/cygwin/fhandler_console.cc | 11 ++++++-----
 winsup/cygwin/fhandler_termios.cc |  2 ++
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index a4c1a3816..2fad7d33c 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1905,6 +1905,8 @@ class fhandler_termios: public fhandler_base
   tty_min *_tc;
   tty *get_ttyp () {return (tty *) tc ();}
   int eat_readahead (int n);
+  virtual void acquire_input_mutex_if_necessary (DWORD ms) {};
+  virtual void release_input_mutex_if_necessary (void) {};
 
  public:
   tty_min*& tc () {return _tc;}
@@ -2212,6 +2214,14 @@ private:
   void __release_input_mutex (const char *fn, int ln);
   DWORD __acquire_output_mutex (const char *fn, int ln, DWORD ms);
   void __release_output_mutex (const char *fn, int ln);
+  void acquire_input_mutex_if_necessary (DWORD ms)
+  {
+    acquire_input_mutex (ms);
+  }
+  void release_input_mutex_if_necessary (void)
+  {
+    release_input_mutex ();
+  }
 
   char *&rabuf ();
   size_t &ralen ();
diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 02d0ac052..0b404411e 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -571,31 +571,34 @@ wait_retry:
 #define buf ((char *) pv)
 
       int ret;
-      acquire_attach_mutex (INFINITE);
       acquire_input_mutex (INFINITE);
+      acquire_attach_mutex (INFINITE);
       ret = process_input_message ();
-      release_input_mutex ();
       release_attach_mutex ();
       switch (ret)
 	{
 	case input_error:
+	  release_input_mutex ();
 	  goto err;
 	case input_processing:
+	  release_input_mutex ();
 	  continue;
 	case input_ok: /* input ready */
 	  break;
 	case input_signalled: /* signalled */
+	  release_input_mutex ();
 	  goto sig_exit;
 	case input_winch:
+	  release_input_mutex ();
 	  continue;
 	default:
 	  /* Should not come here */
+	  release_input_mutex ();
 	  goto err;
 	}
     }
 
   /* Check console read-ahead buffer filled from terminal requests */
-  acquire_input_mutex (INFINITE);
   while (con.cons_rapoi && *con.cons_rapoi && buflen)
     {
       buf[copied_chars++] = *con.cons_rapoi++;
@@ -984,9 +987,7 @@ fhandler_console::process_input_message (void)
       if (toadd)
 	{
 	  ssize_t ret;
-	  release_input_mutex ();
 	  line_edit_status res = line_edit (toadd, nread, *ti, &ret);
-	  acquire_input_mutex (INFINITE);
 	  if (res == line_edit_signalled)
 	    {
 	      stat = input_signalled;
diff --git a/winsup/cygwin/fhandler_termios.cc b/winsup/cygwin/fhandler_termios.cc
index 36bc6255f..9fbace95c 100644
--- a/winsup/cygwin/fhandler_termios.cc
+++ b/winsup/cygwin/fhandler_termios.cc
@@ -332,7 +332,9 @@ fhandler_termios::line_edit (const char *rptr, size_t nread, termios& ti,
 
 	  termios_printf ("got interrupt %d, sending signal %d", c, sig);
 	  eat_readahead (-1);
+	  release_input_mutex_if_necessary ();
 	  tc ()->kill_pgrp (sig);
+	  acquire_input_mutex_if_necessary (INFINITE);
 	  ti.c_lflag &= ~FLUSHO;
 	  sawsig = true;
 	  goto restart_output;
-- 
2.30.0

[PATCH v3 2/2] Cygwin: pty: Make slave read() thread-safe.

[Takashi Yano]

- Currently slave read() is somehow not thread-safe. This patch
  fixes the issue.
---
 winsup/cygwin/fhandler_tty.cc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 06fc19ac2..48b89ae77 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -1241,6 +1241,7 @@ fhandler_pty_slave::read (void *ptr, size_t& len)
 	time_to_wait = !vtime ? INFINITE : 100 * vtime;
     }
 
+wait_retry:
   while (len)
     {
       switch (cygwait (input_available_event, time_to_wait))
@@ -1319,6 +1320,11 @@ fhandler_pty_slave::read (void *ptr, size_t& len)
 	    }
 	  goto out;
 	}
+      if (!IsEventSignalled (input_available_event))
+	{ /* Maybe another thread has processed input. */
+	  ReleaseMutex (input_mutex);
+	  goto wait_retry;
+	}
 
       if (!bytes_available (bytes_in_pipe))
 	{
-- 
2.30.0

Re: [PATCH v3 0/2] Make terminal read() thread-safe.

[Corinna Vinschen]

On Jan 28 23:11, Takashi Yano via Cygwin-patches wrote:
> Currently read() for console and pty slave are somehow
> not thread-safe. These patches fix the issue.
> 
> Takashi Yano (2):
>   Cygwin: console: Make read() thread-safe.
>   Cygwin: pty: Make slave read() thread-safe.
> 
>  winsup/cygwin/fhandler.h          | 10 ++++++++++
>  winsup/cygwin/fhandler_console.cc | 11 ++++++-----
>  winsup/cygwin/fhandler_termios.cc |  2 ++
>  winsup/cygwin/fhandler_tty.cc     |  6 ++++++
>  4 files changed, 24 insertions(+), 5 deletions(-)
> 
> -- 
> 2.30.0

Pushed.

Thanks,
Corinna