public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
From: Takashi Yano <tyan0@sourceware.org>
To: cygwin-cvs@sourceware.org
Subject: [newlib-cygwin/cygwin-3_3-branch] Cygwin: pty: Avoid cutting the branch the pty master is sitting on.
Date: Tue,  1 Mar 2022 10:44:05 +0000 (GMT)	[thread overview]
Message-ID: <20220301104405.AE2AE3858C78@sourceware.org> (raw)

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

commit 598a0c9bbfa818c92df8601f1f377b2eb068090f
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Tue Mar 1 11:34:16 2022 +0900

    Cygwin: pty: Avoid cutting the branch the pty master is sitting on.
    
    - When Ctrl-C terminates a non-cygwin process on a pseudo console,
      pty master attaches to the pseudo console first, and send
      CTRL_C_EVENT. If the non-cygwin process closes the pseudo console
      before the pty master calls FreeConsole(), the pty master process
      will crash. With this patch, pty master process takes over the
      ownership of the pseudo console, and closes it by myself.

Diff:
---
 winsup/cygwin/exceptions.cc       |  3 ++
 winsup/cygwin/fhandler.h          |  2 ++
 winsup/cygwin/fhandler_termios.cc | 20 ++++++++---
 winsup/cygwin/fhandler_tty.cc     | 76 ++++++++++++++++++++-------------------
 winsup/cygwin/sigproc.cc          |  3 +-
 winsup/cygwin/tty.cc              | 11 ++----
 6 files changed, 65 insertions(+), 50 deletions(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 070e52e76..f946bed77 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1157,6 +1157,9 @@ ctrl_c_handler (DWORD type)
 
   tty_min *t = cygwin_shared->tty.get_cttyp ();
 
+  if (!t)
+    return TRUE;
+
   /* If process group leader is non-cygwin process or not exist,
      send signal to myself. */
   pinfo pi (t->getpgid ());
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index d4b08037c..370d9f40b 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2408,6 +2408,8 @@ class fhandler_pty_slave: public fhandler_pty_common
 				 bool stdin_is_ptys);
   static void cleanup_for_non_cygwin_app (handle_set_t *p, tty *ttyp,
 					  bool stdin_is_ptys);
+  static void close_pseudoconsole_if_necessary (tty *ttyp,
+						fhandler_termios *fh);
 };
 
 #define __ptsname(buf, unit) __small_sprintf ((buf), "/dev/pty%d", (unit))
diff --git a/winsup/cygwin/fhandler_termios.cc b/winsup/cygwin/fhandler_termios.cc
index f83770e66..094842038 100644
--- a/winsup/cygwin/fhandler_termios.cc
+++ b/winsup/cygwin/fhandler_termios.cc
@@ -357,6 +357,7 @@ fhandler_termios::process_sigs (char c, tty* ttyp, fhandler_termios *fh)
 	     which the target process is attaching before sending the
 	     CTRL_C_EVENT. After sending the event, reattach to the
 	     console to which the process was previously attached.  */
+	  bool console_exists = fhandler_console::exists ();
 	  pinfo pinfo_resume = pinfo (myself->ppid);
 	  DWORD resume_pid = 0;
 	  if (pinfo_resume)
@@ -364,11 +365,12 @@ fhandler_termios::process_sigs (char c, tty* ttyp, fhandler_termios *fh)
 	  else
 	    resume_pid = fhandler_pty_common::get_console_process_id
 	      (myself->dwProcessId, false);
-	  if (resume_pid && fh && !fh->is_console ())
+	  if ((!console_exists || resume_pid) && fh && !fh->is_console ())
 	    {
 	      FreeConsole ();
 	      AttachConsole (p->dwProcessId);
-	      init_console_handler (true);
+	      init_console_handler (::cygheap->ctty
+				    && ::cygheap->ctty->is_console ());
 	    }
 	  if (fh && p == myself && being_debugged ())
 	    { /* Avoid deadlock in gdb on console. */
@@ -388,11 +390,19 @@ fhandler_termios::process_sigs (char c, tty* ttyp, fhandler_termios *fh)
 	      GenerateConsoleCtrlEvent (CTRL_C_EVENT, 0);
 	      ctrl_c_event_sent = true;
 	    }
-	  if (resume_pid && fh && !fh->is_console ())
+	  if ((!console_exists || resume_pid) && fh && !fh->is_console ())
 	    {
+	      /* If a process on pseudo console is killed by Ctrl-C,
+		 this process may take over the ownership of the
+		 pseudo console because this process attached to it
+		 before sending CTRL_C_EVENT. In this case, closing
+		 pseudo console is necessary. */
+	      fhandler_pty_slave::close_pseudoconsole_if_necessary (ttyp, fh);
 	      FreeConsole ();
-	      AttachConsole (resume_pid);
-	      init_console_handler (true);
+	      if (resume_pid && console_exists)
+		AttachConsole (resume_pid);
+	      init_console_handler (::cygheap->ctty
+				    && ::cygheap->ctty->is_console ());
 	    }
 	  need_discard_input = true;
 	}
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index de5209f40..0ee9bb946 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -537,7 +537,8 @@ fhandler_pty_master::accept_input ()
 	resume_pid = pinfo_resume->dwProcessId;
       else
 	resume_pid = get_console_process_id (myself->dwProcessId, false);
-      if (target_pid && resume_pid)
+      bool console_exists = fhandler_console::exists ();
+      if (target_pid && (resume_pid || !console_exists))
 	{
 	  /* Slave attaches to a different console than master.
 	     Therefore reattach here. */
@@ -546,8 +547,9 @@ fhandler_pty_master::accept_input ()
 	  AttachConsole (target_pid);
 	  cp_to = GetConsoleCP ();
 	  FreeConsole ();
-	  AttachConsole (resume_pid);
-	  init_console_handler (true);
+	  if (resume_pid && console_exists)
+	    AttachConsole (resume_pid);
+	  init_console_handler (false);
 	  release_attach_mutex ();
 	}
       else
@@ -1029,12 +1031,12 @@ fhandler_pty_slave::close ()
   if (!ForceCloseHandle (get_handle_nat ()))
     termios_printf ("CloseHandle (get_handle_nat ()<%p>), %E",
 	get_handle_nat ());
-  if ((unsigned) myself->ctty == FHDEV (DEV_PTYS_MAJOR, get_minor ()))
-    fhandler_console::free_console ();	/* assumes that we are the last pty closer */
   fhandler_pty_common::close ();
   if (!ForceCloseHandle (output_mutex))
     termios_printf ("CloseHandle (output_mutex<%p>), %E", output_mutex);
-  get_ttyp ()->invisible_console_pid = 0;
+  if (get_ttyp ()->invisible_console_pid
+      && !pinfo (get_ttyp ()->invisible_console_pid))
+    get_ttyp ()->invisible_console_pid = 0;
   return 0;
 }
 
@@ -1122,7 +1124,7 @@ pcon_pid_alive (DWORD pid)
 inline static bool
 pcon_pid_self (DWORD pid)
 {
-  return (pid == myself->exec_dwProcessId);
+  return (pid == (myself->exec_dwProcessId ?: myself->dwProcessId));
 }
 
 void
@@ -1240,14 +1242,14 @@ fhandler_pty_slave::reset_switch_to_pcon (void)
 				       0, TRUE, DUPLICATE_SAME_ACCESS);
 		      FreeConsole ();
 		      AttachConsole (get_ttyp ()->pcon_pid);
-		      init_console_handler (true);
+		      init_console_handler (false);
 		      WaitForSingleObject (input_mutex, mutex_timeout);
 		      transfer_input (tty::to_cyg, h_pcon_in, get_ttyp (),
 				      input_available_event);
 		      ReleaseMutex (input_mutex);
 		      FreeConsole ();
 		      AttachConsole (resume_pid);
-		      init_console_handler (true);
+		      init_console_handler (false);
 		      CloseHandle (h_pcon_in);
 		    }
 		  CloseHandle (pcon_owner);
@@ -2839,7 +2841,8 @@ fhandler_pty_master::pty_master_fwd_thread (const master_fwd_thread_param_t *p)
 	resume_pid = pinfo_resume->dwProcessId;
       else
 	resume_pid = get_console_process_id (myself->dwProcessId, false);
-      if (target_pid && resume_pid)
+      bool console_exists = fhandler_console::exists ();
+      if (target_pid && (resume_pid || !console_exists))
 	{
 	  /* Slave attaches to a different console than master.
 	     Therefore reattach here. */
@@ -2848,8 +2851,9 @@ fhandler_pty_master::pty_master_fwd_thread (const master_fwd_thread_param_t *p)
 	  AttachConsole (target_pid);
 	  cp_from = GetConsoleOutputCP ();
 	  FreeConsole ();
-	  AttachConsole (resume_pid);
-	  init_console_handler (true);
+	  if (resume_pid && console_exists)
+	    AttachConsole (resume_pid);
+	  init_console_handler (false);
 	  release_attach_mutex ();
 	}
       else
@@ -3272,7 +3276,7 @@ fhandler_pty_slave::setup_pseudoconsole (bool nopcon)
       CloseHandle (pcon_owner);
       FreeConsole ();
       AttachConsole (get_ttyp ()->pcon_pid);
-      init_console_handler (true);
+      init_console_handler (false);
       goto skip_create;
     }
 
@@ -3396,7 +3400,7 @@ fhandler_pty_slave::setup_pseudoconsole (bool nopcon)
       /* Attach to pseudo console */
       FreeConsole ();
       AttachConsole (pi.dwProcessId);
-      init_console_handler (true);
+      init_console_handler (false);
 
       /* Terminate helper process */
       SetEvent (goodbye);
@@ -3531,6 +3535,8 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to)
       /* Search another process which attaches to the pseudo console */
       DWORD current_pid = myself->exec_dwProcessId ?: myself->dwProcessId;
       switch_to = get_console_process_id (current_pid, false, true, true);
+      if (!switch_to)
+	switch_to = get_console_process_id (current_pid, false, true, false);
     }
   if (ttyp->pcon_activated)
     {
@@ -3579,27 +3585,17 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to)
 	      ttyp->h_pcon_out = new_pcon_out;
 	      FreeConsole ();
 	      pinfo p (myself->ppid);
-	      if (p)
-		{
-		  if (!AttachConsole (p->dwProcessId))
-		    AttachConsole (ATTACH_PARENT_PROCESS);
-		}
-	      else
+	      if (!p || !AttachConsole (p->dwProcessId))
 		AttachConsole (ATTACH_PARENT_PROCESS);
-	      init_console_handler (true);
+	      init_console_handler (false);
 	    }
 	  else
 	    { /* Close pseudo console */
 	      FreeConsole ();
 	      pinfo p (myself->ppid);
-	      if (p)
-		{
-		  if (!AttachConsole (p->dwProcessId))
-		    AttachConsole (ATTACH_PARENT_PROCESS);
-		}
-	      else
+	      if (!p || !AttachConsole (p->dwProcessId))
 		AttachConsole (ATTACH_PARENT_PROCESS);
-	      init_console_handler (true);
+	      init_console_handler (false);
 	      /* Reconstruct pseudo console handler container here for close */
 	      HPCON_INTERNAL *hp =
 		(HPCON_INTERNAL *) HeapAlloc (GetProcessHeap (), 0,
@@ -3621,14 +3617,9 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to)
 	{
 	  FreeConsole ();
 	  pinfo p (myself->ppid);
-	  if (p)
-	    {
-	      if (!AttachConsole (p->dwProcessId))
-		AttachConsole (ATTACH_PARENT_PROCESS);
-	    }
-	  else
+	  if (!p || !AttachConsole (p->dwProcessId))
 	    AttachConsole (ATTACH_PARENT_PROCESS);
-	  init_console_handler (true);
+	  init_console_handler (false);
 	}
     }
   else if (pcon_pid_self (ttyp->pcon_pid))
@@ -3795,7 +3786,7 @@ fhandler_pty_slave::create_invisible_console ()
       /* Detach from console device and create new invisible console. */
       FreeConsole();
       fhandler_console::need_invisible (true);
-      init_console_handler (true);
+      init_console_handler (false);
       get_ttyp ()->need_invisible_console = false;
       get_ttyp ()->invisible_console_pid = myself->pid;
     }
@@ -4082,3 +4073,16 @@ fhandler_pty_master::need_send_ctrl_c_event ()
   return !(to_be_read_from_pcon () && get_ttyp ()->pcon_activated
     && get_ttyp ()->pcon_input_state == tty::to_nat);
 }
+
+void
+fhandler_pty_slave::close_pseudoconsole_if_necessary (tty *ttyp,
+						      fhandler_termios *fh)
+{
+  if (fh->get_major () == DEV_PTYM_MAJOR && ttyp->pcon_activated)
+    {
+      fhandler_pty_master *ptym = (fhandler_pty_master *) fh;
+      WaitForSingleObject (ptym->pcon_mutex, INFINITE);
+      close_pseudoconsole (ttyp);
+      ReleaseMutex (ptym->pcon_mutex);
+    }
+}
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 02d875a7f..dc77af61b 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -1412,7 +1412,8 @@ wait_sig (VOID *)
 	  sig_held = true;
 	  break;
 	case __SIGSETPGRP:
-	  init_console_handler (true);
+	  init_console_handler (::cygheap->ctty
+				&& ::cygheap->ctty->is_console ());
 	  break;
 	case __SIGTHREADEXIT:
 	  {
diff --git a/winsup/cygwin/tty.cc b/winsup/cygwin/tty.cc
index a98808855..1782be6c5 100644
--- a/winsup/cygwin/tty.cc
+++ b/winsup/cygwin/tty.cc
@@ -344,7 +344,7 @@ tty_min::setpgid (int pid)
 	      CloseHandle (pcon_owner);
 	      FreeConsole ();
 	      AttachConsole (ttyp->pcon_pid);
-	      init_console_handler (true);
+	      init_console_handler (false);
 	      attach_restore = true;
 	    }
 	  WaitForSingleObject (ptys->input_mutex, mutex_timeout);
@@ -355,14 +355,9 @@ tty_min::setpgid (int pid)
 	    {
 	      FreeConsole ();
 	      pinfo p (myself->ppid);
-	      if (p)
-		{
-		  if (!AttachConsole (p->dwProcessId))
-		    AttachConsole (ATTACH_PARENT_PROCESS);
-		}
-	      else
+	      if (!p || !AttachConsole (p->dwProcessId))
 		AttachConsole (ATTACH_PARENT_PROCESS);
-	      init_console_handler (true);
+	      init_console_handler (false);
 	    }
 	}
       ReleaseMutex (ptys->pcon_mutex);


                 reply	other threads:[~2022-03-01 10:44 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220301104405.AE2AE3858C78@sourceware.org \
    --to=tyan0@sourceware.org \
    --cc=cygwin-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).