public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
From: Takashi Yano <takashi.yano@nifty.ne.jp>
To: cygwin-patches@cygwin.com
Subject: [PATCH] Cygwin: pipe: Restore non-blocking mode which was reset for non-cygwin app.
Date: Sun, 10 Mar 2024 19:31:52 +0900	[thread overview]
Message-ID: <20240310103202.3753-1-takashi.yano@nifty.ne.jp> (raw)

If pipe reader is a non-cygwin app first, and cygwin process reads
the same pipe after that, the pipe has been set to bclocking mode
for the cygwin app. Hoever, the commit 9e4d308cd592 assumes the pipe
for cygwin process always is non-blocking mode. With this patch,
the pipe mode is restored to non-blocking when the non-cygwin app
exits. As the side effect of this fix, query_hdl for non-cygwin app
can be retrieved from its stub process. Therefore, 46bb999a894 has
been modified a bit as well.

Addresses: https://cygwin.com/pipermail/cygwin/2024-March/255644.html
Fixes: 9e4d308cd592 ("Cygwin: pipe: Adopt FILE_SYNCHRONOUS_IO_NONALERT flag for read pipe.")
Reported-by: wh <wh9692@protonmail.com>
Reviewed-by:
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
 winsup/cygwin/fhandler/pipe.cc | 12 +++++++-----
 winsup/cygwin/spawn.cc         | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc
index 29d3b41d9..11c833297 100644
--- a/winsup/cygwin/fhandler/pipe.cc
+++ b/winsup/cygwin/fhandler/pipe.cc
@@ -1205,15 +1205,17 @@ fhandler_pipe::get_query_hdl_per_process (OBJECT_NAME_INFORMATION *ntfn)
       ULONG len;
 
       /* Non-cygwin app may call ReadFile() for empty pipe, which makes
-	NtQueryObject() for ObjectNameInformation block. Therefore, do
-	not try to get query_hdl for non-cygwin apps. */
+	NtQueryObject() for ObjectNameInformation block. However, the stub
+	process for the non-cygwin app should have dup'ed pipe handle.
+	Therefore, use exec_dwProcessId instead. */
       _pinfo *p = pids[i];
-      if (!p || ISSTATE (p, PID_NOTCYGWIN))
+      if (!p)
 	continue;
 
+      DWORD winpid =
+	ISSTATE (p, PID_NOTCYGWIN) ? p->exec_dwProcessId : p->dwProcessId;
       HANDLE proc = OpenProcess (PROCESS_DUP_HANDLE
-				 | PROCESS_QUERY_INFORMATION,
-				 0, p->dwProcessId);
+				 | PROCESS_QUERY_INFORMATION, 0, winpid);
       if (!proc)
 	continue;
 
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 71d75bbf4..92723e5a1 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -579,6 +579,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
       int fileno_stdout = in__stdout < 0 ? 1 : in__stdout;
       int fileno_stderr = 2;
 
+      fhandler_pipe *pipew_duped = NULL, *piper_duped = NULL;
       if (!iscygwin ())
 	{
 	  bool need_send_sig = false;
@@ -590,6 +591,10 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	      {
 		fhandler_pipe *pipe = (fhandler_pipe *)(fhandler_base *) cfd;
 		pipe->set_pipe_non_blocking (false);
+		pipew_duped = (fhandler_pipe *)
+			ccalloc (HEAP_FHANDLER, 1, sizeof (fhandler_pipe));
+		pipew_duped = new (pipew_duped) fhandler_pipe;
+		pipe->dup (pipew_duped, 0);
 		if (pipe->request_close_query_hdl ())
 		  need_send_sig = true;
 	      }
@@ -597,6 +602,10 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	      {
 		fhandler_pipe *pipe = (fhandler_pipe *)(fhandler_base *) cfd;
 		pipe->set_pipe_non_blocking (false);
+		piper_duped = (fhandler_pipe *)
+			ccalloc (HEAP_FHANDLER, 1, sizeof (fhandler_pipe));
+		piper_duped = new (piper_duped) fhandler_pipe;
+		pipe->dup (piper_duped, 0);
 	      }
 
 	  if (need_send_sig)
@@ -905,6 +914,19 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	      term_spawn_worker.cleanup ();
 	      term_spawn_worker.close_handle_set ();
 	    }
+	  if (pipew_duped)
+	    {
+	      bool is_nonblocking = pipew_duped->is_nonblocking ();
+	      pipew_duped->set_pipe_non_blocking (is_nonblocking);
+	      pipew_duped->close ();
+	      cfree (pipew_duped);
+	    }
+	  if (piper_duped)
+	    {
+	      piper_duped->set_pipe_non_blocking (true);
+	      piper_duped->close ();
+	      cfree (piper_duped);
+	    }
 	  /* Make sure that ctrl_c_handler() is not on going. Calling
 	     init_console_handler(false) locks until returning from
 	     ctrl_c_handler(). This insures that setting sigExeced
-- 
2.43.0


             reply	other threads:[~2024-03-10 10:32 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-10 10:31 Takashi Yano [this message]
2024-03-11 10:47 ` Corinna Vinschen
2024-03-11 11:42   ` Takashi Yano
2024-03-11 13:18     ` Takashi Yano
2024-03-11 20:33       ` Corinna Vinschen
2024-03-11 23:03         ` Takashi Yano
2024-03-11 23:17           ` Takashi Yano
2024-03-12 10:53             ` Corinna Vinschen

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=20240310103202.3753-1-takashi.yano@nifty.ne.jp \
    --to=takashi.yano@nifty.ne.jp \
    --cc=cygwin-patches@cygwin.com \
    /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).