From 61b20e8d469eb82292ef4ca885d824f429fe4b2a Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 29 Aug 2013 15:04:27 +0200 Subject: [PATCH] windows: make uv_shutdown() for write-only pipes work A couple of issues prevented uv_shutdown() from working correctly with write-only pipes. * The pipe handle wasn't opened with the right permissions, so an attempt to probe the state of the write buffer would fail with ERROR_ACCESS_DENIED. * The pipe flags for child process stdio pipes were always set to UV_HANDLE_READABLE and UV_HANDLE_WRITABLE, even in cases where it was actually half-duplex. * There was no code path that lead to closing the pipe handle if the pipe was write-only. --- src/win/pipe.c | 10 ++++++++-- src/win/process-stdio.c | 15 +++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/win/pipe.c b/src/win/pipe.c index 0fb70eae..fd7418d7 100644 --- a/src/win/pipe.c +++ b/src/win/pipe.c @@ -1528,9 +1528,9 @@ void uv_process_pipe_shutdown_req(uv_loop_t* loop, uv_pipe_t* handle, UNREGISTER_HANDLE_REQ(loop, handle, req); - /* Initialize and optionally start the eof timer. */ - /* This makes no sense if we've already seen EOF. */ if (handle->flags & UV_HANDLE_READABLE) { + /* Initialize and optionally start the eof timer. Only do this if the */ + /* pipe is readable and we haven't seen EOF come in ourselves. */ eof_timer_init(handle); /* If reading start the timer right now. */ @@ -1538,6 +1538,12 @@ void uv_process_pipe_shutdown_req(uv_loop_t* loop, uv_pipe_t* handle, if (handle->flags & UV_HANDLE_READ_PENDING) { eof_timer_start(handle); } + + } else { + /* This pipe is not readable. We can just close it to let the other end */ + /* know that we're done writing. */ + CloseHandle(handle->handle); + handle->handle = INVALID_HANDLE_VALUE; } if (req->cb) { diff --git a/src/win/process-stdio.c b/src/win/process-stdio.c index 89e2cd99..e9cf84da 100644 --- a/src/win/process-stdio.c +++ b/src/win/process-stdio.c @@ -104,12 +104,16 @@ static uv_err_t uv__create_stdio_pipe_pair(uv_loop_t* loop, uv_err_t err; if (flags & UV_READABLE_PIPE) { - server_access |= PIPE_ACCESS_OUTBOUND; + /* The server needs inbound access too, otherwise CreateNamedPipe() */ + /* won't give us the FILE_READ_ATTRIBUTES permission. We need that to */ + /* probe the state of the write buffer when we're trying to shutdown */ + /* the pipe. */ + server_access |= PIPE_ACCESS_OUTBOUND | PIPE_ACCESS_INBOUND; client_access |= GENERIC_READ | FILE_WRITE_ATTRIBUTES; } if (flags & UV_WRITABLE_PIPE) { server_access |= PIPE_ACCESS_INBOUND; - client_access |= GENERIC_WRITE; + client_access |= GENERIC_WRITE | FILE_READ_ATTRIBUTES; } /* Create server pipe handle. */ @@ -163,8 +167,11 @@ static uv_err_t uv__create_stdio_pipe_pair(uv_loop_t* loop, } } - /* The server end is now readable and writable. */ - server_pipe->flags |= UV_HANDLE_READABLE | UV_HANDLE_WRITABLE; + /* The server end is now readable and/or writable. */ + if (flags & UV_READABLE_PIPE) + server_pipe->flags |= UV_HANDLE_WRITABLE; + if (flags & UV_WRITABLE_PIPE) + server_pipe->flags |= UV_HANDLE_READABLE; *child_pipe_ptr = child_pipe; return uv_ok_;