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.
joyent · Aug 29, 2013 · b4332f8 · bnoordhuis · Aug 29, 2013 · b4332f8
1 parent 82f2472
commit b4332f8
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 6 deletions.
diff --git a/src/win/pipe.c b/src/win/pipe.c
@@ -1594,16 +1594,22 @@ 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 seend EOF come in ourselves. */
     eof_timer_init(handle);
 
     /* If reading start the timer right now. */
     /* Otherwise uv_pipe_queue_read will start it. */
     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
@@ -104,12 +104,16 @@ static int uv__create_stdio_pipe_pair(uv_loop_t* loop,
   int 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 int 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 0;