public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
From: Corinna Vinschen <corinna@sourceware.org>
To: cygwin-cvs@sourceware.org
Subject: [newlib-cygwin] Cygwin: make sure exec'ed process exists early in process list
Date: Thu, 19 May 2022 10:06:38 +0000 (GMT)	[thread overview]
Message-ID: <20220519100638.104A33856DD6@sourceware.org> (raw)

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

commit 1b86dd7d8c193bca46f08c8f900ae9678c8331d5
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Thu May 19 10:46:33 2022 +0200

    Cygwin: make sure exec'ed process exists early in process list
    
    killpg(pgid, 0) (or kill_pgrp(pgid, si_signo=0), in signal.cc)
    fails (returns -1) even when there is a process in the process
    group pgid, if the process is in the middle of spawnve(), see
    
      https://cygwin.com/pipermail/cygwin/2022-May/251479.html
    
    When exec'ing a process the assumption is that the exec'ed process creates its
    own symlink (in pinfo::thisproc() in pinfo.cc). If the exec'ing process
    calls NtClose on it's own winpid symlink, but the exec'ed process didn't
    progress enough into initialization, there's a slim chance that neither
    the exec'ing process, nor the exec'ed process has a winpid symlink
    attached.
    
    Always create the winpid symlink in spawn.cc, even for exec'ed Cygwin
    processes.  Make sure to dup the handle into the new process, and stop
    creating the winpid symlink in exec'ed processes.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/pinfo.cc      |  6 ++++--
 winsup/cygwin/release/3.3.6 |  4 ++++
 winsup/cygwin/spawn.cc      | 15 ++++++++-------
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 4a8daefd9..f501c470f 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -55,10 +55,11 @@ void
 pinfo::thisproc (HANDLE h)
 {
   procinfo = NULL;
+  bool execed = !!h;
 
   DWORD flags = PID_IN_USE | PID_ACTIVE;
   /* Forked process or process started from non-Cygwin parent needs a pid. */
-  if (!h)
+  if (!execed)
     {
       cygheap->pid = create_cygwin_pid ();
       flags |= PID_NEW;
@@ -72,7 +73,8 @@ pinfo::thisproc (HANDLE h)
   procinfo->dwProcessId = myself_initial.dwProcessId;
   procinfo->sendsig = myself_initial.sendsig;
   wcscpy (procinfo->progname, myself_initial.progname);
-  create_winpid_symlink ();
+  if (!execed)
+    create_winpid_symlink ();
   procinfo->exec_sendsig = NULL;
   procinfo->exec_dwProcessId = 0;
   debug_printf ("myself dwProcessId %u", procinfo->dwProcessId);
diff --git a/winsup/cygwin/release/3.3.6 b/winsup/cygwin/release/3.3.6
index 6d1f0bc8b..6d722433f 100644
--- a/winsup/cygwin/release/3.3.6
+++ b/winsup/cygwin/release/3.3.6
@@ -3,3 +3,7 @@ Bug Fixes
 
 - Fix an issue that command "cmd /c script -c cmd" crashes if it
   is issued in console of Windows 7.
+
+- Fix killpg failing because the exec'ing as well as the exec'ed
+  process are not in the pidlist for a brief moment.
+  Addresses: https://cygwin.com/pipermail/cygwin/2022-May/251479.html
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 98b588698..905d949fc 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -860,13 +860,14 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	  strace.execing = 1;
 	  myself.hProcess = hExeced = pi.hProcess;
 	  HANDLE old_winpid_hdl = myself.shared_winpid_handle ();
-	  if (!real_path.iscygexec ())
-	    {
-	      /* If the child process is not a Cygwin process, we have to
-		 create a new winpid symlink on behalf of the child process
-		 not being able to do this by itself. */
-	      myself.create_winpid_symlink ();
-	    }
+	  /* We have to create a new winpid symlink on behalf of the child
+	     process. For Cygwin processes we also have to create a reference
+	     in the child. */
+	  myself.create_winpid_symlink ();
+	  if (real_path.iscygexec ())
+	    DuplicateHandle (GetCurrentProcess (),
+			     myself.shared_winpid_handle (),
+			     pi.hProcess, NULL, 0, 0, DUPLICATE_SAME_ACCESS);
 	  NtClose (old_winpid_hdl);
 	  real_path.get_wide_win32_path (myself->progname); // FIXME: race?
 	  sigproc_printf ("new process name %W", myself->progname);


                 reply	other threads:[~2022-05-19 10:06 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=20220519100638.104A33856DD6@sourceware.org \
    --to=corinna@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).