public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
* [newlib-cygwin] Revert "Cygwin: pipe: Revert to create() rather than nt_create()."
@ 2021-09-14 15:05 Corinna Vinschen
0 siblings, 0 replies; only message in thread
From: Corinna Vinschen @ 2021-09-14 15:05 UTC (permalink / raw)
To: cygwin-cvs
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=24b7a74b9481cca9772735d2296e6cec5e7cb204
commit 24b7a74b9481cca9772735d2296e6cec5e7cb204
Author: Ken Brown <kbrown@cornell.edu>
Date: Mon Aug 30 09:53:06 2021 -0400
Revert "Cygwin: pipe: Revert to create() rather than nt_create()."
This reverts commit 5a7a0d34c74a55aa1e76644e61bf4889051cb640.
Diff:
---
winsup/cygwin/fhandler_pipe.cc | 192 ++++++++++++++++++++++++++++++++++++-----
1 file changed, 171 insertions(+), 21 deletions(-)
diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index a6ece17a6..2dec0a848 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -463,13 +463,13 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w,
if (name)
len += __small_sprintf (pipename + len, "%s", name);
- open_mode |= PIPE_ACCESS_OUTBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE;
+ open_mode |= PIPE_ACCESS_INBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE;
/* Retry CreateNamedPipe as long as the pipe name is in use.
Retrying will probably never be necessary, but we want
to be as robust as possible. */
DWORD err = 0;
- while (w && !*w)
+ while (r && !*r)
{
static volatile ULONG pipe_unique_id;
if (!name)
@@ -498,12 +498,12 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w,
definitely required for pty handling since fhandler_pty_master
writes to the pipe in chunks, terminated by newline when CANON mode
is specified. */
- *w = CreateNamedPipe (pipename, open_mode, pipe_mode, 1, psize,
+ *r = CreateNamedPipe (pipename, open_mode, pipe_mode, 1, psize,
psize, NMPWAIT_USE_DEFAULT_WAIT, sa_ptr);
- if (*w != INVALID_HANDLE_VALUE)
+ if (*r != INVALID_HANDLE_VALUE)
{
- debug_printf ("pipe write handle %p", *w);
+ debug_printf ("pipe read handle %p", *r);
err = 0;
break;
}
@@ -516,14 +516,14 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w,
Pick a new name and retry. */
debug_printf ("pipe busy", !name ? ", retrying" : "");
if (!name)
- *w = NULL;
+ *r = NULL;
break;
case ERROR_ACCESS_DENIED:
/* The pipe is already open with incompatible parameters.
Pick a new name and retry. */
debug_printf ("pipe access denied%s", !name ? ", retrying" : "");
if (!name)
- *w = NULL;
+ *r = NULL;
break;
default:
{
@@ -535,42 +535,60 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w,
if (err)
{
- *w = NULL;
+ *r = NULL;
return err;
}
- if (!r)
- debug_printf ("pipe read handle NULL");
+ if (!w)
+ debug_printf ("pipe write handle NULL");
else
{
debug_printf ("CreateFile: name %s", pipename);
- /* Open the named pipe for reading.
- Be sure to permit FILE_WRITE_ATTRIBUTES access. */
- DWORD access = GENERIC_READ | FILE_WRITE_ATTRIBUTES;
+ /* Open the named pipe for writing.
+ Be sure to permit FILE_READ_ATTRIBUTES access. */
+ DWORD access = GENERIC_WRITE | FILE_READ_ATTRIBUTES;
if ((open_mode & PIPE_ACCESS_DUPLEX) == PIPE_ACCESS_DUPLEX)
- access |= GENERIC_WRITE | FILE_READ_ATTRIBUTES;
- *r = CreateFile (pipename, access, 0, sa_ptr, OPEN_EXISTING,
+ access |= GENERIC_READ | FILE_WRITE_ATTRIBUTES;
+ *w = CreateFile (pipename, access, 0, sa_ptr, OPEN_EXISTING,
open_mode & FILE_FLAG_OVERLAPPED, 0);
- if (!*r || *r == INVALID_HANDLE_VALUE)
+ if (!*w || *w == INVALID_HANDLE_VALUE)
{
/* Failure. */
DWORD err = GetLastError ();
debug_printf ("CreateFile failed, r %p, %E", r);
- if (w)
- CloseHandle (*w);
- *r = NULL;
+ if (r)
+ CloseHandle (*r);
+ *w = NULL;
return err;
}
- debug_printf ("pipe read handle %p", *r);
+ debug_printf ("pipe write handle %p", *w);
}
/* Success. */
return 0;
}
+/* The next version of fhandler_pipe::create used to call the previous
+ version. But the read handle created by the latter doesn't have
+ FILE_WRITE_ATTRIBUTES access unless the pipe is created with
+ PIPE_ACCESS_DUPLEX, and it doesn't seem possible to add that access
+ right. This causes set_pipe_non_blocking to fail.
+
+ To fix this we will define a helper function 'nt_create' that is
+ similar to the above fhandler_pipe::create but uses
+ NtCreateNamedPipeFile instead of CreateNamedPipe; this gives more
+ flexibility in setting the access rights. We will use this helper
+ function in the version of fhandler_pipe::create below, which
+ suffices for all of our uses of set_pipe_non_blocking. For
+ simplicity, nt_create will omit the 'open_mode' and 'name'
+ parameters, which aren't needed for our purposes. */
+
+static int nt_create (LPSECURITY_ATTRIBUTES, PHANDLE, PHANDLE, DWORD,
+ int64_t *);
+
int
fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode)
{
@@ -579,7 +597,7 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode)
int res = -1;
int64_t unique_id;
- int ret = create (sa, &r, &w, psize, NULL, 0, &unique_id);
+ int ret = nt_create (sa, &r, &w, psize, &unique_id);
if (ret)
__seterrno_from_win_error (ret);
else if ((fhs[0] = (fhandler_pipe *) build_fh_dev (*piper_dev)) == NULL)
@@ -606,6 +624,138 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode)
return res;
}
+static int
+nt_create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w,
+ DWORD psize, int64_t *unique_id)
+{
+ NTSTATUS status;
+ HANDLE npfsh;
+ ACCESS_MASK access;
+ OBJECT_ATTRIBUTES attr;
+ IO_STATUS_BLOCK io;
+ LARGE_INTEGER timeout;
+
+ /* Default to error. */
+ if (r)
+ *r = NULL;
+ if (w)
+ *w = NULL;
+
+ status = fhandler_base::npfs_handle (npfsh);
+ if (!NT_SUCCESS (status))
+ {
+ __seterrno_from_nt_status (status);
+ return GetLastError ();
+ }
+
+ /* Ensure that there is enough pipe buffer space for atomic writes. */
+ if (!psize)
+ psize = DEFAULT_PIPEBUFSIZE;
+
+ UNICODE_STRING pipename;
+ WCHAR pipename_buf[MAX_PATH];
+ size_t len = __small_swprintf (pipename_buf, L"%S-%u-",
+ &cygheap->installation_key,
+ GetCurrentProcessId ());
+
+ access = GENERIC_READ | FILE_WRITE_ATTRIBUTES;
+
+ ULONG pipe_type = pipe_byte ? FILE_PIPE_BYTE_STREAM_TYPE
+ : FILE_PIPE_MESSAGE_TYPE;
+
+ /* Retry NtCreateNamedPipeFile as long as the pipe name is in use.
+ Retrying will probably never be necessary, but we want
+ to be as robust as possible. */
+ DWORD err = 0;
+ while (r && !*r)
+ {
+ static volatile ULONG pipe_unique_id;
+ LONG id = InterlockedIncrement ((LONG *) &pipe_unique_id);
+ __small_swprintf (pipename_buf + len, L"pipe-nt-%p", id);
+ if (unique_id)
+ *unique_id = ((int64_t) id << 32 | GetCurrentProcessId ());
+
+ debug_printf ("name %W, size %u, mode %s", pipename_buf, psize,
+ (pipe_type & FILE_PIPE_MESSAGE_TYPE)
+ ? "PIPE_TYPE_MESSAGE" : "PIPE_TYPE_BYTE");
+
+ RtlInitUnicodeString (&pipename, pipename_buf);
+
+ InitializeObjectAttributes (&attr, &pipename,
+ sa_ptr->bInheritHandle ? OBJ_INHERIT : 0,
+ npfsh, sa_ptr->lpSecurityDescriptor);
+
+ timeout.QuadPart = -500000;
+ status = NtCreateNamedPipeFile (r, access, &attr, &io,
+ FILE_SHARE_READ | FILE_SHARE_WRITE,
+ FILE_CREATE, 0, pipe_type,
+ FILE_PIPE_BYTE_STREAM_MODE,
+ 0, 1, psize, psize, &timeout);
+
+ if (NT_SUCCESS (status))
+ {
+ debug_printf ("pipe read handle %p", *r);
+ err = 0;
+ break;
+ }
+
+ switch (status)
+ {
+ case STATUS_PIPE_BUSY:
+ case STATUS_INSTANCE_NOT_AVAILABLE:
+ case STATUS_PIPE_NOT_AVAILABLE:
+ /* The pipe is already open with compatible parameters.
+ Pick a new name and retry. */
+ debug_printf ("pipe busy, retrying");
+ *r = NULL;
+ break;
+ case STATUS_ACCESS_DENIED:
+ /* The pipe is already open with incompatible parameters.
+ Pick a new name and retry. */
+ debug_printf ("pipe access denied, retrying");
+ *r = NULL;
+ break;
+ default:
+ {
+ __seterrno_from_nt_status (status);
+ err = GetLastError ();
+ debug_printf ("failed, %E");
+ *r = INVALID_HANDLE_VALUE;
+ }
+ }
+ }
+
+ if (err)
+ {
+ *r = NULL;
+ return err;
+ }
+
+ if (!w)
+ debug_printf ("pipe write handle NULL");
+ else
+ {
+ debug_printf ("NtOpenFile: name %S", &pipename);
+
+ access = GENERIC_WRITE | FILE_READ_ATTRIBUTES;
+ status = NtOpenFile (w, access, &attr, &io, 0, 0);
+ if (!NT_SUCCESS (status))
+ {
+ DWORD err = GetLastError ();
+ debug_printf ("NtOpenFile failed, r %p, %E", r);
+ if (r)
+ CloseHandle (*r);
+ *w = NULL;
+ return err;
+ }
+
+ debug_printf ("pipe write handle %p", *w);
+ }
+
+ /* Success. */
+ return 0;
+}
+
int
fhandler_pipe::ioctl (unsigned int cmd, void *p)
{
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2021-09-14 15:05 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-14 15:05 [newlib-cygwin] Revert "Cygwin: pipe: Revert to create() rather than nt_create()." 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).