From f43f1a7016f86b868dbb8f3377930647f06f1f4b Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 8 Mar 2012 16:43:56 +0100 Subject: [PATCH 01/16] Windows: avoid uv_guess_handle crash in when fd < 0 Happens only when using a debug version of the crt. --- src/win/handle.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/win/handle.c b/src/win/handle.c index ba0af755..4f37f4c0 100644 --- a/src/win/handle.c +++ b/src/win/handle.c @@ -27,9 +27,15 @@ uv_handle_type uv_guess_handle(uv_file file) { - HANDLE handle = (HANDLE) _get_osfhandle(file); + HANDLE handle; DWORD mode; + if (file < 0) { + return UV_UNKNOWN_HANDLE; + } + + handle = (HANDLE) _get_osfhandle(file); + switch (GetFileType(handle)) { case FILE_TYPE_CHAR: if (GetConsoleMode(handle, &mode)) { From 743cab9f9d4aa13d7e3b79a0aaf63341bd6e26e6 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 8 Mar 2012 16:44:30 +0100 Subject: [PATCH 02/16] Test runner: avoid process_wait failure when the test process didn't start --- test/runner.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/runner.c b/test/runner.c index fdd81684..73570fc3 100644 --- a/test/runner.c +++ b/test/runner.c @@ -186,7 +186,8 @@ out: process_terminate(&processes[i]); } - if (process_wait(processes, process_count - 1, -1) < 0) { + if (process_count > 0 && + process_wait(processes, process_count - 1, -1) < 0) { FATAL("process_wait failed"); } From e53d7e3a11df84b1faf4100d96d5fb94052bff13 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Thu, 8 Mar 2012 16:45:46 +0100 Subject: [PATCH 03/16] Make test-tty pass with redirected stdio --- test/test-tty.c | 64 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/test/test-tty.c b/test/test-tty.c index 1e3e1f28..24f0def6 100644 --- a/test/test-tty.c +++ b/test/test-tty.c @@ -22,23 +22,64 @@ #include "uv.h" #include "task.h" +#ifdef _WIN32 +# include +# include +#else // Unix +# include +# include +#endif + + TEST_IMPL(tty) { int r, width, height; - uv_tty_t tty; + int ttyin_fd, ttyout_fd; + uv_tty_t tty_in, tty_out; uv_loop_t* loop = uv_default_loop(); + // Make sure we have an FD that refers to a tty +#ifdef _WIN32 + HANDLE handle; + handle = CreateFileA("conin$", + GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + NULL); + ASSERT(handle != INVALID_HANDLE_VALUE); + ttyin_fd = _open_osfhandle((intptr_t) handle, 0); + + handle = CreateFileA("conout$", + GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL, + NULL); + ASSERT(handle != INVALID_HANDLE_VALUE); + ttyout_fd = _open_osfhandle((intptr_t) handle, 0); + +#else /* unix */ + ttyin_fd = open("/dev/tty", O_RDONLY, 0); + ttyout_fd = open("/dev/tty", O_WRONLY, 0); +#endif + + ASSERT(ttyin_fd >= 0); + ASSERT(ttyout_fd >= 0); + ASSERT(UV_UNKNOWN_HANDLE == uv_guess_handle(-1)); - /* - * Not necessarily a problem if this assert goes off. E.G you are piping - * this test to a file. 0 == stdin. - */ - ASSERT(UV_TTY == uv_guess_handle(0)); + ASSERT(UV_TTY == uv_guess_handle(ttyin_fd)); + ASSERT(UV_TTY == uv_guess_handle(ttyout_fd)); - r = uv_tty_init(uv_default_loop(), &tty, 0, 1); + r = uv_tty_init(uv_default_loop(), &tty_in, ttyin_fd, 1); ASSERT(r == 0); - r = uv_tty_get_winsize(&tty, &width, &height); + r = uv_tty_init(uv_default_loop(), &tty_out, ttyout_fd, 2); + ASSERT(r == 0); + + r = uv_tty_get_winsize(&tty_out, &width, &height); ASSERT(r == 0); printf("width=%d height=%d\n", width, height); @@ -51,16 +92,17 @@ TEST_IMPL(tty) { ASSERT(height > 10); /* Turn on raw mode. */ - r = uv_tty_set_mode(&tty, 1); + r = uv_tty_set_mode(&tty_in, 1); ASSERT(r == 0); /* Turn off raw mode. */ - r = uv_tty_set_mode(&tty, 0); + r = uv_tty_set_mode(&tty_in, 0); ASSERT(r == 0); /* TODO check the actual mode! */ - uv_close((uv_handle_t*)&tty, NULL); + uv_close((uv_handle_t*) &tty_in, NULL); + uv_close((uv_handle_t*) &tty_out, NULL); uv_run(loop); From 87752ac38bc5d6ca0cb12504ef07e2c2ab2a617d Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:59:21 +0100 Subject: [PATCH 04/16] Fold trailing whitespace --- src/win/pipe.c | 10 +++++----- src/win/process.c | 2 +- src/win/tcp.c | 8 ++++---- test/run-tests.c | 6 +++--- test/test-fs.c | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/win/pipe.c b/src/win/pipe.c index ae769c1d..cb49b92b 100644 --- a/src/win/pipe.c +++ b/src/win/pipe.c @@ -338,7 +338,7 @@ void uv_pipe_endgame(uv_loop_t* loop, uv_pipe_t* handle) { free(handle->pending_socket_info); handle->pending_socket_info = NULL; } - + if (handle->flags & UV_HANDLE_EMULATE_IOCP) { if (handle->read_req.wait_handle != INVALID_HANDLE_VALUE) { UnregisterWait(handle->read_req.wait_handle); @@ -866,7 +866,7 @@ static void uv_pipe_queue_read(uv_loop_t* loop, uv_pipe_t* handle) { /* Make this req pending reporting an error. */ SET_REQ_ERROR(req, GetLastError()); goto error; - } + } } else { memset(&req->overlapped, 0, sizeof(req->overlapped)); if (handle->flags & UV_HANDLE_EMULATE_IOCP) { @@ -1071,7 +1071,7 @@ static int uv_pipe_write_impl(uv_loop_t* loop, uv_write_t* req, ipc_frame.header.raw_data_length = bufs[0].len; } - /* + /* * Use the provided req if we're only doing a single write. * If we're doing multiple writes, use ipc_header_write_req to do * the first write, and then use the provided req for the second write. @@ -1079,7 +1079,7 @@ static int uv_pipe_write_impl(uv_loop_t* loop, uv_write_t* req, if (!(ipc_frame.header.flags & UV_IPC_RAW_DATA)) { ipc_header_req = req; } else { - /* + /* * Try to use the preallocated write req if it's available. * Otherwise allocate a new one. */ @@ -1359,7 +1359,7 @@ void uv_process_pipe_read_req(uv_loop_t* loop, uv_pipe_t* handle, /* Successful read */ if (handle->ipc) { assert(handle->remaining_ipc_rawdata_bytes >= bytes); - handle->remaining_ipc_rawdata_bytes = + handle->remaining_ipc_rawdata_bytes = handle->remaining_ipc_rawdata_bytes - bytes; if (handle->read2_cb) { handle->read2_cb(handle, bytes, buf, diff --git a/src/win/process.c b/src/win/process.c index 3d649835..a23ba0b1 100644 --- a/src/win/process.c +++ b/src/win/process.c @@ -832,7 +832,7 @@ done: static int duplicate_std_handle(uv_loop_t* loop, DWORD id, HANDLE* dup) { HANDLE handle; HANDLE current_process = GetCurrentProcess(); - + handle = GetStdHandle(id); if (handle == NULL) { diff --git a/src/win/tcp.c b/src/win/tcp.c index 628d294d..1ca618c3 100644 --- a/src/win/tcp.c +++ b/src/win/tcp.c @@ -552,7 +552,7 @@ int uv_tcp_accept(uv_tcp_t* server, uv_tcp_t* client) { if (server->processed_accepts >= uv_simultaneous_server_accepts) { server->processed_accepts = 0; - /* + /* * All previously queued accept requests are now processed. * We now switch to queueing just a single accept. */ @@ -835,7 +835,7 @@ void uv_process_tcp_read_req(uv_loop_t* loop, uv_tcp_t* handle, err = GET_REQ_SOCK_ERROR(req); if (err == WSAECONNABORTED) { - /* + /* * Turn WSAECONNABORTED into UV_ECONNRESET to be consistent with Unix. */ uv__set_error(loop, UV_ECONNRESET, err); @@ -904,7 +904,7 @@ void uv_process_tcp_read_req(uv_loop_t* loop, uv_tcp_t* handle, handle->read_cb((uv_stream_t*)handle, 0, buf); } else { if (err == WSAECONNABORTED) { - /* + /* * Turn WSAECONNABORTED into UV_ECONNRESET to be consistent with Unix. */ uv__set_error(loop, UV_ECONNRESET, err); @@ -1090,7 +1090,7 @@ int uv_tcp_duplicate_socket(uv_tcp_t* handle, int pid, LPWSAPROTOCOL_INFOW protocol_info) { assert(!(handle->flags & UV_HANDLE_CONNECTION)); - /* + /* * We're about to share the socket with another process. Because * this is a listening socket, we assume that the other process will * be accepting connections on it. So, before sharing the socket diff --git a/test/run-tests.c b/test/run-tests.c index a3e08fc0..fc92d7b7 100644 --- a/test/run-tests.c +++ b/test/run-tests.c @@ -84,7 +84,7 @@ static void ipc_on_connection(uv_stream_t* server, int status) { uv_tcp_t* conn; if (!connection_accepted) { - /* + /* * Accept the connection and close it. Also let the other * side know. */ @@ -119,7 +119,7 @@ static int ipc_helper(int listen_after_write) { * data is transfered over the channel. XXX edit this comment after handle * transfer is added. */ - + uv_write_t write_req; int r; uv_buf_t buf; @@ -206,7 +206,7 @@ static int stdio_over_pipes_helper() { uv_buf_t buf[COUNTOF(buffers)]; int r, i; uv_loop_t* loop = uv_default_loop(); - + ASSERT(UV_NAMED_PIPE == uv_guess_handle(0)); ASSERT(UV_NAMED_PIPE == uv_guess_handle(1)); diff --git a/test/test-fs.c b/test/test-fs.c index af2ebbf5..6d63ba97 100644 --- a/test/test-fs.c +++ b/test/test-fs.c @@ -115,7 +115,7 @@ void check_permission(const char* filename, int mode) { s = req.ptr; #ifdef _WIN32 - /* + /* * On Windows, chmod can only modify S_IWUSR (_S_IWRITE) bit, * so only testing for the specified flags. */ @@ -1210,7 +1210,7 @@ TEST_IMPL(fs_symlink) { */ return 0; } else if (uv_last_error(loop).sys_errno_ == ERROR_PRIVILEGE_NOT_HELD) { - /* + /* * Creating a symlink is only allowed when running elevated. * We pass the test and bail out early if we get ERROR_PRIVILEGE_NOT_HELD. */ From ec97ba801469d797d468c06d4db7c5e0afb6d0ac Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:25:08 +0100 Subject: [PATCH 05/16] Windows uv_fs_stat: cap st_nlink to SHRT_MAX --- src/win/fs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/win/fs.c b/src/win/fs.c index 9c112b80..507336ee 100644 --- a/src/win/fs.c +++ b/src/win/fs.c @@ -547,7 +547,8 @@ static void fs__stat(uv_fs_t* req, const wchar_t* path) { req->stat.st_size = ((int64_t) info.nFileSizeHigh << 32) + (int64_t) info.nFileSizeLow; - req->stat.st_nlink = info.nNumberOfLinks; + req->stat.st_nlink = (info.nNumberOfLinks <= SHRT_MAX) ? + (short) info.nNumberOfLinks : SHRT_MAX; req->ptr = &req->stat; req->result = 0; From fb65d74c84d88405446bcea87a9f5199728cc5b3 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:26:51 +0100 Subject: [PATCH 06/16] Tests: verify that uv_write and uv_shutdown ref the event loop --- test/test-list.h | 12 ++++++++ test/test-ref.c | 76 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/test/test-list.h b/test/test-list.h index 13713717..033be382 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -79,12 +79,16 @@ TEST_DECLARE (fs_event_ref) TEST_DECLARE (tcp_ref) TEST_DECLARE (tcp_ref2) TEST_DECLARE (tcp_ref3) +TEST_DECLARE (tcp_ref4) +TEST_DECLARE (tcp_ref5) TEST_DECLARE (udp_ref) TEST_DECLARE (udp_ref2) TEST_DECLARE (udp_ref3) TEST_DECLARE (pipe_ref) TEST_DECLARE (pipe_ref2) TEST_DECLARE (pipe_ref3) +TEST_DECLARE (pipe_ref4) +TEST_DECLARE (pipe_ref5) TEST_DECLARE (process_ref) TEST_DECLARE (async) TEST_DECLARE (get_currentexe) @@ -228,6 +232,10 @@ TASK_LIST_START TEST_ENTRY (tcp_ref2) TEST_ENTRY (tcp_ref3) TEST_HELPER (tcp_ref3, tcp4_echo_server) + TEST_ENTRY (tcp_ref4) + TEST_HELPER (tcp_ref4, tcp4_echo_server) + TEST_ENTRY (tcp_ref5) + TEST_HELPER (tcp_ref5, tcp4_echo_server) TEST_ENTRY (udp_ref) TEST_ENTRY (udp_ref2) TEST_ENTRY (udp_ref3) @@ -236,6 +244,10 @@ TASK_LIST_START TEST_ENTRY (pipe_ref2) TEST_ENTRY (pipe_ref3) TEST_HELPER (pipe_ref3, pipe_echo_server) + TEST_ENTRY (pipe_ref4) + TEST_HELPER (pipe_ref4, pipe_echo_server) + TEST_ENTRY (pipe_ref5) + TEST_HELPER (pipe_ref5, pipe_echo_server) TEST_ENTRY (process_ref) TEST_ENTRY (loop_handles) diff --git a/test/test-ref.c b/test/test-ref.c index 2b8aabbc..9a474b56 100644 --- a/test/test-ref.c +++ b/test/test-ref.c @@ -26,11 +26,39 @@ #include +static uv_write_t write_req; +static uv_shutdown_t shutdown_req; +static uv_connect_t connect_req; + +static char buffer[32767]; + + static void fail_cb(void) { FATAL("fail_cb should not have been called"); } +static void write_unref_cb(uv_connect_t* req, int status) { + uv_buf_t buf = uv_buf_init(buffer, sizeof buffer); + + ASSERT(req == &connect_req); + ASSERT(status == 0); + + uv_write(&write_req, req->handle, &buf, 1, (uv_write_cb) fail_cb); + uv_unref(uv_default_loop()); // uv_write refs the loop +} + + + +static void shutdown_unref_cb(uv_connect_t* req, int status) { + ASSERT(req == &connect_req); + ASSERT(status == 0); + + uv_shutdown(&shutdown_req, req->handle, (uv_shutdown_cb) fail_cb); + uv_unref(uv_default_loop()); // uv_shutdown refs the loop +} + + TEST_IMPL(ref) { uv_run(uv_default_loop()); return 0; @@ -142,10 +170,9 @@ TEST_IMPL(tcp_ref2) { TEST_IMPL(tcp_ref3) { struct sockaddr_in addr = uv_ip4_addr("127.0.0.1", TEST_PORT); - uv_connect_t req; uv_tcp_t h; uv_tcp_init(uv_default_loop(), &h); - uv_tcp_connect(&req, &h, addr, (uv_connect_cb)fail_cb); + uv_tcp_connect(&connect_req, &h, addr, (uv_connect_cb)fail_cb); uv_unref(uv_default_loop()); uv_unref(uv_default_loop()); /* connect req refs the loop */ uv_run(uv_default_loop()); @@ -153,6 +180,28 @@ TEST_IMPL(tcp_ref3) { } +TEST_IMPL(tcp_ref4) { + struct sockaddr_in addr = uv_ip4_addr("127.0.0.1", TEST_PORT); + uv_tcp_t h; + uv_tcp_init(uv_default_loop(), &h); + uv_tcp_connect(&connect_req, &h, addr, write_unref_cb); + uv_unref(uv_default_loop()); + uv_run(uv_default_loop()); + return 0; +} + + +TEST_IMPL(tcp_ref5) { + struct sockaddr_in addr = uv_ip4_addr("127.0.0.1", TEST_PORT); + uv_tcp_t h; + uv_tcp_init(uv_default_loop(), &h); + uv_tcp_connect(&connect_req, &h, addr, shutdown_unref_cb); + uv_unref(uv_default_loop()); + uv_run(uv_default_loop()); + return 0; +} + + TEST_IMPL(udp_ref) { uv_udp_t h; uv_udp_init(uv_default_loop(), &h); @@ -210,10 +259,9 @@ TEST_IMPL(pipe_ref2) { TEST_IMPL(pipe_ref3) { - uv_connect_t req; uv_pipe_t h; uv_pipe_init(uv_default_loop(), &h, 0); - uv_pipe_connect(&req, &h, TEST_PIPENAME, (uv_connect_cb)fail_cb); + uv_pipe_connect(&connect_req, &h, TEST_PIPENAME, (uv_connect_cb)fail_cb); uv_unref(uv_default_loop()); uv_unref(uv_default_loop()); /* connect req refs the loop */ uv_run(uv_default_loop()); @@ -221,6 +269,26 @@ TEST_IMPL(pipe_ref3) { } +TEST_IMPL(pipe_ref4) { + uv_pipe_t h; + uv_pipe_init(uv_default_loop(), &h, 0); + uv_pipe_connect(&connect_req, &h, TEST_PIPENAME, write_unref_cb); + uv_unref(uv_default_loop()); + uv_run(uv_default_loop()); + return 0; +} + + +TEST_IMPL(pipe_ref5) { + uv_pipe_t h; + uv_pipe_init(uv_default_loop(), &h, 0); + uv_pipe_connect(&connect_req, &h, TEST_PIPENAME, shutdown_unref_cb); + uv_unref(uv_default_loop()); + uv_run(uv_default_loop()); + return 0; +} + + TEST_IMPL(process_ref) { /* spawn_helper4 blocks indefinitely. */ char *argv[] = { NULL, "spawn_helper4", NULL }; From 422a898a7f77ba0180dd3fe2b32738ba413b2e86 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:29:45 +0100 Subject: [PATCH 07/16] Tests: verify that shutdown_cb is always called --- test/test-list.h | 7 +++ test/test-shutdown-close.c | 101 +++++++++++++++++++++++++++++++++++++ uv.gyp | 1 + 3 files changed, 109 insertions(+) create mode 100644 test/test-shutdown-close.c diff --git a/test/test-list.h b/test/test-list.h index 033be382..9e5309c1 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -59,6 +59,8 @@ TEST_DECLARE (pipe_listen_without_bind) TEST_DECLARE (pipe_connect_bad_name) TEST_DECLARE (connection_fail) TEST_DECLARE (connection_fail_doesnt_auto_close) +TEST_DECLARE (shutdown_close_tcp) +TEST_DECLARE (shutdown_close_pipe) TEST_DECLARE (shutdown_eof) TEST_DECLARE (callback_stack) TEST_DECLARE (error_message) @@ -206,6 +208,11 @@ TASK_LIST_START TEST_ENTRY (connection_fail) TEST_ENTRY (connection_fail_doesnt_auto_close) + TEST_ENTRY (shutdown_close_tcp) + TEST_HELPER (shutdown_close_tcp, tcp4_echo_server) + TEST_ENTRY (shutdown_close_pipe) + TEST_HELPER (shutdown_close_pipe, pipe_echo_server) + TEST_ENTRY (shutdown_eof) TEST_HELPER (shutdown_eof, tcp4_echo_server) diff --git a/test/test-shutdown-close.c b/test/test-shutdown-close.c new file mode 100644 index 00000000..864f7cec --- /dev/null +++ b/test/test-shutdown-close.c @@ -0,0 +1,101 @@ +/* Copyright Joyent, Inc. and other Node contributors. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to + * deal in the Software without restriction, including without limitation the + * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + */ + +/* + * These tests verify that the uv_shutdown callback is always made, even when + * it is immediately followed by an uv_close call. + */ + +#include "uv.h" +#include "task.h" + + +static uv_shutdown_t shutdown_req; +static uv_connect_t connect_req; + +static int connect_cb_called = 0; +static int shutdown_cb_called = 0; +static int close_cb_called = 0; + + +static void shutdown_cb(uv_shutdown_t* req, int status) { + ASSERT(req == &shutdown_req); + ASSERT(status == 0 || + (status == -1 && uv_last_error(uv_default_loop()).code == UV_EINTR)); + shutdown_cb_called++; +} + + +static void close_cb(uv_handle_t* handle) { + close_cb_called++; +} + + +static void connect_cb(uv_connect_t* req, int status) { + int r; + + ASSERT(req == &connect_req); + ASSERT(status == 0); + + r = uv_shutdown(&shutdown_req, req->handle, shutdown_cb); + ASSERT(r == 0); + uv_close((uv_handle_t*) req->handle, close_cb); + + connect_cb_called++; +} + + +TEST_IMPL(shutdown_close_tcp) { + struct sockaddr_in addr = uv_ip4_addr("127.0.0.1", TEST_PORT); + uv_tcp_t h; + int r; + + r = uv_tcp_init(uv_default_loop(), &h); + ASSERT(r == 0); + r = uv_tcp_connect(&connect_req, &h, addr, connect_cb); + ASSERT(r == 0); + r = uv_run(uv_default_loop()); + ASSERT(r == 0); + + ASSERT(connect_cb_called == 1); + ASSERT(shutdown_cb_called == 1); + ASSERT(close_cb_called == 1); + + return 0; +} + + +TEST_IMPL(shutdown_close_pipe) { + uv_pipe_t h; + int r; + + r = uv_pipe_init(uv_default_loop(), &h, 0); + ASSERT(r == 0); + uv_pipe_connect(&connect_req, &h, TEST_PIPENAME, connect_cb); + r = uv_run(uv_default_loop()); + ASSERT(r == 0); + + ASSERT(connect_cb_called == 1); + ASSERT(shutdown_cb_called == 1); + ASSERT(close_cb_called == 1); + + return 0; +} diff --git a/uv.gyp b/uv.gyp index 07ea07a8..8151878f 100644 --- a/uv.gyp +++ b/uv.gyp @@ -299,6 +299,7 @@ 'test/test-pipe-bind-error.c', 'test/test-pipe-connect-error.c', 'test/test-ref.c', + 'test/test-shutdown-close.c', 'test/test-shutdown-eof.c', 'test/test-spawn.c', 'test/test-stdio-over-pipes.c', From 09a0d61e7bd4965f0a2d9219943b7eff9c3d9944 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:31:55 +0100 Subject: [PATCH 08/16] Test cwd_and_chdir: don't chdir to "" or "c:" It should not strip the trailing (back)slash from a root directory. --- test/test-cwd-and-chdir.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/test/test-cwd-and-chdir.c b/test/test-cwd-and-chdir.c index deafdc94..d72c26a8 100644 --- a/test/test-cwd-and-chdir.c +++ b/test/test-cwd-and-chdir.c @@ -37,17 +37,20 @@ TEST_IMPL(cwd_and_chdir) { err = uv_cwd(buffer_orig, size); ASSERT(err.code == UV_OK); - last_slash = strrchr(buffer_orig, + /* Remove trailing slash unless at a root directory. */ #ifdef _WIN32 - '\\' -#else - '/' -#endif - ); - + last_slash = strrchr(buffer_orig, '\\'); ASSERT(last_slash); - - *last_slash = '\0'; + if (last_slash > buffer_orig && *(last_slash - 1) != ':') { + *last_slash = '\0'; + } +#else /* Unix */ + last_slash = strrchr(buffer_orig, '/'); + ASSERT(last_slash); + if (last_slash != buffer_orig) { + *last_slash = '\0'; + } +#endif err = uv_chdir(buffer_orig); ASSERT(err.code == UV_OK); From abc4f56ff02a33ac318bcef4281c64b3167db1de Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:32:24 +0100 Subject: [PATCH 09/16] Test chown_root: make it pass on windows --- test/test-fs.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/test-fs.c b/test/test-fs.c index 6d63ba97..8dc71473 100644 --- a/test/test-fs.c +++ b/test/test-fs.c @@ -186,8 +186,14 @@ static void chown_cb(uv_fs_t* req) { static void chown_root_cb(uv_fs_t* req) { ASSERT(req->fs_type == UV_FS_CHOWN); +#ifdef _WIN32 + /* On windows, chown is a no-op and always succeeds. */ + ASSERT(req->result == 0); +#else + /* On unix, chown'ing the root directory is not allowed. */ ASSERT(req->result == -1); ASSERT(req->errorno == UV_EPERM); +#endif chown_cb_count++; uv_fs_req_cleanup(req); } From aafa7db1d4ba7ad7645a3008335bf30cd272277a Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:33:04 +0100 Subject: [PATCH 10/16] Test benchmark_pound: fix compilation problem --- test/benchmark-pound.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/benchmark-pound.c b/test/benchmark-pound.c index 64f404ca..1693c31c 100644 --- a/test/benchmark-pound.c +++ b/test/benchmark-pound.c @@ -107,7 +107,9 @@ static void connect_cb(uv_connect_t* req, int status) { if (status != 0) { #if DEBUG - fprintf(stderr, "connect error %s\n", uv_err_name(uv_last_error())); + fprintf(stderr, + "connect error %s\n", + uv_err_name(uv_last_error(uv_default_loop()))); #endif uv_close((uv_handle_t*)req->handle, close_cb); conns_failed++; From f285caf0b53de01c2a0d4591467bffb7134a81c7 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:33:33 +0100 Subject: [PATCH 11/16] Test counters_init: fix compiler warnings --- test/test-counters-init.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/test-counters-init.c b/test/test-counters-init.c index 423d349c..29e7acdf 100644 --- a/test/test-counters-init.c +++ b/test/test-counters-init.c @@ -92,21 +92,21 @@ static void create_cb(uv_fs_t* req) { TEST_IMPL(counters_init) { int r; - int eio_init_prev; - int req_init_prev; - int handle_init_prev; - int stream_init_prev; - int tcp_init_prev; - int udp_init_prev; - int pipe_init_prev; - int tty_init_prev; - int prepare_init_prev; - int check_init_prev; - int idle_init_prev; - int async_init_prev; - int timer_init_prev; - int process_init_prev; - int fs_event_init_prev; + uint64_t eio_init_prev; + uint64_t req_init_prev; + uint64_t handle_init_prev; + uint64_t stream_init_prev; + uint64_t tcp_init_prev; + uint64_t udp_init_prev; + uint64_t pipe_init_prev; + uint64_t tty_init_prev; + uint64_t prepare_init_prev; + uint64_t check_init_prev; + uint64_t idle_init_prev; + uint64_t async_init_prev; + uint64_t timer_init_prev; + uint64_t process_init_prev; + uint64_t fs_event_init_prev; /* req_init and eio_init test by uv_fs_open() */ unlink("test_file"); From 50216706c2014ad6d8ee3bd48e63e1a97790057b Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:34:02 +0100 Subject: [PATCH 12/16] Test runner: fix compiler warnings --- test/run-tests.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/run-tests.c b/test/run-tests.c index fc92d7b7..a8c88652 100644 --- a/test/run-tests.c +++ b/test/run-tests.c @@ -129,8 +129,8 @@ static int ipc_helper(int listen_after_write) { uv_pipe_open(&channel, 0); - ASSERT(uv_is_readable(&channel)); - ASSERT(uv_is_writable(&channel)); + ASSERT(uv_is_readable((uv_stream_t*) &channel)); + ASSERT(uv_is_writable((uv_stream_t*) &channel)); r = uv_tcp_init(uv_default_loop(), &tcp_server); ASSERT(r == 0); From 3aa6069abf0e36eb7d41eb078fadf7299b7a7918 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:37:05 +0100 Subject: [PATCH 13/16] Windows test runner: show process name instead of test name If a test failed we would previously see: Output from process `test_foo`: blah Output from process `test_foo`: (nothing) This commit changes it to: Output from process `test_foo`: blah Output from process `foo_helper`: (nothing) --- test/runner-win.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/runner-win.c b/test/runner-win.c index 6b6d500c..d2a986a7 100644 --- a/test/runner-win.c +++ b/test/runner-win.c @@ -135,7 +135,7 @@ int process_start(char *name, char *part, process_info_t *p) { p->stdio_in = nul; p->stdio_out = file; p->process = pi.hProcess; - p->name = name; + p->name = part; return 0; From 18823270aa800da0e54eb86ec08e22dfe3e7e5fb Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:42:21 +0100 Subject: [PATCH 14/16] Windows: fix connecting a pipe in the thread pool The code didn't function. Fixes the pipe_pound benchmarks. --- src/win/pipe.c | 113 +++++++++++++++++++++++++---------------------- src/win/stream.c | 2 +- 2 files changed, 62 insertions(+), 53 deletions(-) diff --git a/src/win/pipe.c b/src/win/pipe.c index cb49b92b..0b12525f 100644 --- a/src/win/pipe.c +++ b/src/win/pipe.c @@ -98,59 +98,61 @@ static void uv_pipe_connection_init(uv_pipe_t* handle) { } -static int open_named_pipe(uv_pipe_t* handle) { - /* - * Assume that we have a duplex pipe first, so attempt to +static HANDLE open_named_pipe(WCHAR* name, DWORD* duplex_flags) { + HANDLE pipeHandle; + + /* + * Assume that we have a duplex pipe first, so attempt to * connect with GENERIC_READ | GENERIC_WRITE. */ - handle->handle = CreateFileW(handle->name, - GENERIC_READ | GENERIC_WRITE, - 0, - NULL, - OPEN_EXISTING, - FILE_FLAG_OVERLAPPED, - NULL); - - if (handle->handle != INVALID_HANDLE_VALUE) { - return 0; + pipeHandle = CreateFileW(name, + GENERIC_READ | GENERIC_WRITE, + 0, + NULL, + OPEN_EXISTING, + FILE_FLAG_OVERLAPPED, + NULL); + if (pipeHandle != INVALID_HANDLE_VALUE) { + *duplex_flags = 0; + return pipeHandle; } - /* - * If the pipe is not duplex CreateFileW fails with + /* + * If the pipe is not duplex CreateFileW fails with * ERROR_ACCESS_DENIED. In that case try to connect * as a read-only or write-only. */ if (GetLastError() == ERROR_ACCESS_DENIED) { - handle->handle = CreateFileW(handle->name, - GENERIC_READ | FILE_WRITE_ATTRIBUTES, - 0, - NULL, - OPEN_EXISTING, - FILE_FLAG_OVERLAPPED, - NULL); + pipeHandle = CreateFileW(name, + GENERIC_READ | FILE_WRITE_ATTRIBUTES, + 0, + NULL, + OPEN_EXISTING, + FILE_FLAG_OVERLAPPED, + NULL); - if (handle->handle != INVALID_HANDLE_VALUE) { - handle->flags |= UV_HANDLE_SHUT; - return 0; + if (pipeHandle != INVALID_HANDLE_VALUE) { + *duplex_flags = UV_HANDLE_SHUTTING; + return pipeHandle; } } if (GetLastError() == ERROR_ACCESS_DENIED) { - handle->handle = CreateFileW(handle->name, - GENERIC_WRITE | FILE_READ_ATTRIBUTES, - 0, - NULL, - OPEN_EXISTING, - FILE_FLAG_OVERLAPPED, - NULL); + pipeHandle = CreateFileW(name, + GENERIC_WRITE | FILE_READ_ATTRIBUTES, + 0, + NULL, + OPEN_EXISTING, + FILE_FLAG_OVERLAPPED, + NULL); - if (handle->handle != INVALID_HANDLE_VALUE) { - handle->flags |= UV_HANDLE_EOF; - return 0; + if (pipeHandle != INVALID_HANDLE_VALUE) { + *duplex_flags = UV_HANDLE_EOF; + return pipeHandle; } } - return -1; + return INVALID_HANDLE_VALUE; } @@ -208,7 +210,7 @@ done: static int uv_set_pipe_handle(uv_loop_t* loop, uv_pipe_t* handle, - HANDLE pipeHandle) { + HANDLE pipeHandle, DWORD duplex_flags) { NTSTATUS nt_status; IO_STATUS_BLOCK io_status; FILE_MODE_INFORMATION mode_info; @@ -242,6 +244,9 @@ static int uv_set_pipe_handle(uv_loop_t* loop, uv_pipe_t* handle, } } + handle->handle = pipeHandle; + handle->flags |= duplex_flags; + return 0; } @@ -449,7 +454,7 @@ int uv_pipe_bind(uv_pipe_t* handle, const char* name) { goto error; } - if (uv_set_pipe_handle(loop, handle, handle->accept_reqs[0].pipeHandle)) { + if (uv_set_pipe_handle(loop, handle, handle->accept_reqs[0].pipeHandle, 0)) { uv__set_sys_error(loop, GetLastError()); goto error; } @@ -476,11 +481,12 @@ error: static DWORD WINAPI pipe_connect_thread_proc(void* parameter) { - HANDLE pipeHandle = INVALID_HANDLE_VALUE; int errno; uv_loop_t* loop; uv_pipe_t* handle; uv_connect_t* req; + HANDLE pipeHandle = INVALID_HANDLE_VALUE; + DWORD duplex_flags; req = (uv_connect_t*) parameter; assert(req); @@ -493,7 +499,8 @@ static DWORD WINAPI pipe_connect_thread_proc(void* parameter) { /* We wait for the pipe to become available with WaitNamedPipe. */ while (WaitNamedPipeW(handle->name, 30000)) { /* The pipe is now available, try to connect. */ - if (open_named_pipe(handle) == 0) { + pipeHandle = open_named_pipe(handle->name, &duplex_flags); + if (pipeHandle != INVALID_HANDLE_VALUE) { break; } @@ -501,8 +508,7 @@ static DWORD WINAPI pipe_connect_thread_proc(void* parameter) { } if (pipeHandle != INVALID_HANDLE_VALUE && - !uv_set_pipe_handle(loop, handle, pipeHandle)) { - handle->handle = pipeHandle; + !uv_set_pipe_handle(loop, handle, pipeHandle, duplex_flags)) { SET_REQ_SUCCESS(req); } else { SET_REQ_ERROR(req, GetLastError()); @@ -519,8 +525,8 @@ void uv_pipe_connect(uv_connect_t* req, uv_pipe_t* handle, const char* name, uv_connect_cb cb) { uv_loop_t* loop = handle->loop; int errno, nameSize; - - handle->handle = INVALID_HANDLE_VALUE; + HANDLE pipeHandle = INVALID_HANDLE_VALUE; + DWORD duplex_flags; uv_req_init(loop, (uv_req_t*) req); req->type = UV_CONNECT; @@ -539,7 +545,8 @@ void uv_pipe_connect(uv_connect_t* req, uv_pipe_t* handle, goto error; } - if (open_named_pipe(handle) != 0) { + pipeHandle = open_named_pipe(handle->name, &duplex_flags); + if (pipeHandle == INVALID_HANDLE_VALUE) { if (GetLastError() == ERROR_PIPE_BUSY) { /* Wait for the server to make a pipe instance available. */ if (!QueueUserWorkItem(&pipe_connect_thread_proc, @@ -558,9 +565,12 @@ void uv_pipe_connect(uv_connect_t* req, uv_pipe_t* handle, goto error; } - assert(handle->handle != INVALID_HANDLE_VALUE); + assert(pipeHandle != INVALID_HANDLE_VALUE); - if (uv_set_pipe_handle(loop, (uv_pipe_t*)req->handle, handle->handle)) { + if (uv_set_pipe_handle(loop, + (uv_pipe_t*) req->handle, + pipeHandle, + duplex_flags)) { errno = GetLastError(); goto error; } @@ -576,9 +586,8 @@ error: handle->name = NULL; } - if (handle->handle != INVALID_HANDLE_VALUE) { - CloseHandle(handle->handle); - handle->handle = INVALID_HANDLE_VALUE; + if (pipeHandle != INVALID_HANDLE_VALUE) { + CloseHandle(pipeHandle); } /* Make this req pending reporting an error. */ @@ -643,7 +652,7 @@ static void uv_pipe_queue_accept(uv_loop_t* loop, uv_pipe_t* handle, return; } - if (uv_set_pipe_handle(loop, handle, req->pipeHandle)) { + if (uv_set_pipe_handle(loop, handle, req->pipeHandle, 0)) { CloseHandle(req->pipeHandle); req->pipeHandle = INVALID_HANDLE_VALUE; SET_REQ_ERROR(req, GetLastError()); @@ -1612,7 +1621,7 @@ void uv_pipe_open(uv_pipe_t* pipe, uv_file file) { HANDLE os_handle = (HANDLE)_get_osfhandle(file); if (os_handle == INVALID_HANDLE_VALUE || - uv_set_pipe_handle(pipe->loop, pipe, os_handle) == -1) { + uv_set_pipe_handle(pipe->loop, pipe, os_handle, 0) == -1) { return; } diff --git a/src/win/stream.c b/src/win/stream.c index ea7363ef..21da1b1a 100644 --- a/src/win/stream.c +++ b/src/win/stream.c @@ -194,5 +194,5 @@ int uv_is_readable(uv_stream_t* handle) { int uv_is_writable(uv_stream_t* handle) { - return !(handle->flags & UV_HANDLE_SHUT); + return !(handle->flags & UV_HANDLE_SHUTTING); } From 95296dfa3cd8c88b3af8a94c95b8df51a6367c37 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:52:48 +0100 Subject: [PATCH 15/16] Windows: make the refcount tests pass --- src/win/handle.c | 2 +- src/win/pipe.c | 24 ++++++++++++------------ src/win/stream.c | 1 + src/win/tcp.c | 9 +++++++++ src/win/tty.c | 3 +++ src/win/udp.c | 3 +++ 6 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/win/handle.c b/src/win/handle.c index 4f37f4c0..a4eadbdd 100644 --- a/src/win/handle.c +++ b/src/win/handle.c @@ -179,7 +179,7 @@ void uv_want_endgame(uv_loop_t* loop, uv_handle_t* handle) { void uv_process_endgames(uv_loop_t* loop) { uv_handle_t* handle; - while (loop->endgame_handles) { + while (loop->endgame_handles && loop->refs > 0) { handle = loop->endgame_handles; loop->endgame_handles = handle->endgame_next; diff --git a/src/win/pipe.c b/src/win/pipe.c index 0b12525f..ab86018d 100644 --- a/src/win/pipe.c +++ b/src/win/pipe.c @@ -300,6 +300,7 @@ void uv_pipe_endgame(uv_loop_t* loop, uv_pipe_t* handle) { uv__set_sys_error(loop, pRtlNtStatusToDosError(nt_status)); req->cb(req, -1); } + uv_unref(loop); DECREASE_PENDING_REQ_COUNT(handle); return; } @@ -328,6 +329,7 @@ void uv_pipe_endgame(uv_loop_t* loop, uv_pipe_t* handle) { uv__set_sys_error(loop, GetLastError()); req->cb(req, -1); } + uv_unref(loop); DECREASE_PENDING_REQ_COUNT(handle); return; } @@ -556,6 +558,7 @@ void uv_pipe_connect(uv_connect_t* req, uv_pipe_t* handle, goto error; } + uv_ref(loop); handle->reqs_pending++; return; @@ -578,6 +581,7 @@ void uv_pipe_connect(uv_connect_t* req, uv_pipe_t* handle, SET_REQ_SUCCESS(req); uv_insert_pending_req(loop, (uv_req_t*) req); handle->reqs_pending++; + uv_ref(loop); return; error: @@ -594,6 +598,7 @@ error: SET_REQ_ERROR(req, errno); uv_insert_pending_req(loop, (uv_req_t*) req); handle->reqs_pending++; + uv_ref(loop); return; } @@ -1132,10 +1137,7 @@ static int uv_pipe_write_impl(uv_loop_t* loop, uv_write_t* req, handle->write_queue_size += req->queued_bytes; } - if (handle->write_reqs_pending == 0) { - uv_ref(loop); - } - + uv_ref(loop); handle->reqs_pending++; handle->write_reqs_pending++; @@ -1190,10 +1192,7 @@ static int uv_pipe_write_impl(uv_loop_t* loop, uv_write_t* req, } } - if (handle->write_reqs_pending == 0) { - uv_ref(loop); - } - + uv_ref(loop); handle->reqs_pending++; handle->write_reqs_pending++; @@ -1449,15 +1448,12 @@ void uv_process_pipe_write_req(uv_loop_t* loop, uv_pipe_t* handle, uv_queue_non_overlapped_write(handle); } - if (handle->write_reqs_pending == 0) { - uv_unref(loop); - } - if (handle->write_reqs_pending == 0 && handle->flags & UV_HANDLE_SHUTTING) { uv_want_endgame(loop, (uv_handle_t*)handle); } + uv_unref(loop); DECREASE_PENDING_REQ_COUNT(handle); } @@ -1504,6 +1500,7 @@ void uv_process_pipe_connect_req(uv_loop_t* loop, uv_pipe_t* handle, } } + uv_unref(loop); DECREASE_PENDING_REQ_COUNT(handle); } @@ -1528,6 +1525,7 @@ void uv_process_pipe_shutdown_req(uv_loop_t* loop, uv_pipe_t* handle, req->cb(req, 0); } + uv_unref(loop); DECREASE_PENDING_REQ_COUNT(handle); } @@ -1543,6 +1541,7 @@ static void eof_timer_init(uv_pipe_t* pipe) { r = uv_timer_init(pipe->loop, pipe->eof_timer); assert(r == 0); /* timers can't fail */ pipe->eof_timer->data = pipe; + uv_unref(pipe->loop); } @@ -1605,6 +1604,7 @@ static void eof_timer_destroy(uv_pipe_t* pipe) { assert(pipe->flags && UV_HANDLE_CONNECTION); if (pipe->eof_timer) { + uv_ref(pipe->loop); uv_close((uv_handle_t*) pipe->eof_timer, eof_timer_close_cb); pipe->eof_timer = NULL; } diff --git a/src/win/stream.c b/src/win/stream.c index 21da1b1a..53c4d9ec 100644 --- a/src/win/stream.c +++ b/src/win/stream.c @@ -169,6 +169,7 @@ int uv_shutdown(uv_shutdown_t* req, uv_stream_t* handle, uv_shutdown_cb cb) { handle->flags |= UV_HANDLE_SHUTTING; handle->shutdown_req = req; handle->reqs_pending++; + uv_ref(loop); uv_want_endgame(loop, (uv_handle_t*)handle); diff --git a/src/win/tcp.c b/src/win/tcp.c index 1ca618c3..31de8448 100644 --- a/src/win/tcp.c +++ b/src/win/tcp.c @@ -189,6 +189,7 @@ void uv_tcp_endgame(uv_loop_t* loop, uv_tcp_t* handle) { handle->shutdown_req->cb(handle->shutdown_req, status); } + uv_unref(loop); DECREASE_PENDING_REQ_COUNT(handle); return; } @@ -643,10 +644,12 @@ int uv__tcp_connect(uv_connect_t* req, if (UV_SUCCEEDED_WITHOUT_IOCP(success)) { /* Process the req without IOCP. */ handle->reqs_pending++; + uv_ref(loop); uv_insert_pending_req(loop, (uv_req_t*)req); } else if (UV_SUCCEEDED_WITH_IOCP(success)) { /* The req will be processed with IOCP. */ handle->reqs_pending++; + uv_ref(loop); } else { uv__set_sys_error(loop, WSAGetLastError()); return -1; @@ -702,9 +705,11 @@ int uv__tcp_connect6(uv_connect_t* req, if (UV_SUCCEEDED_WITHOUT_IOCP(success)) { handle->reqs_pending++; + uv_ref(loop); uv_insert_pending_req(loop, (uv_req_t*)req); } else if (UV_SUCCEEDED_WITH_IOCP(success)) { handle->reqs_pending++; + uv_ref(loop); } else { uv__set_sys_error(loop, WSAGetLastError()); return -1; @@ -799,12 +804,14 @@ int uv_tcp_write(uv_loop_t* loop, uv_write_t* req, uv_tcp_t* handle, handle->reqs_pending++; handle->write_reqs_pending++; uv_insert_pending_req(loop, (uv_req_t*) req); + uv_ref(loop); } else if (UV_SUCCEEDED_WITH_IOCP(result == 0)) { /* Request queued by the kernel. */ req->queued_bytes = uv_count_bufs(bufs, bufcnt); handle->reqs_pending++; handle->write_reqs_pending++; handle->write_queue_size += req->queued_bytes; + uv_ref(loop); } else { /* Send failed due to an error. */ uv__set_sys_error(loop, WSAGetLastError()); @@ -950,6 +957,7 @@ void uv_process_tcp_write_req(uv_loop_t* loop, uv_tcp_t* handle, } DECREASE_PENDING_REQ_COUNT(handle); + uv_unref(loop); } @@ -1024,6 +1032,7 @@ void uv_process_tcp_connect_req(uv_loop_t* loop, uv_tcp_t* handle, } DECREASE_PENDING_REQ_COUNT(handle); + uv_unref(loop); } diff --git a/src/win/tty.c b/src/win/tty.c index 556875d4..2a696c89 100644 --- a/src/win/tty.c +++ b/src/win/tty.c @@ -1683,6 +1683,7 @@ int uv_tty_write(uv_loop_t* loop, uv_write_t* req, uv_tty_t* handle, handle->reqs_pending++; handle->write_reqs_pending++; + uv_ref(loop); req->queued_bytes = 0; @@ -1715,6 +1716,7 @@ void uv_process_tty_write_req(uv_loop_t* loop, uv_tty_t* handle, } DECREASE_PENDING_REQ_COUNT(handle); + uv_unref(loop); } @@ -1740,6 +1742,7 @@ void uv_tty_endgame(uv_loop_t* loop, uv_tty_t* handle) { handle->shutdown_req->cb(handle->shutdown_req, 0); } + uv_unref(loop); DECREASE_PENDING_REQ_COUNT(handle); return; } diff --git a/src/win/udp.c b/src/win/udp.c index 39221de6..18ae4a2c 100644 --- a/src/win/udp.c +++ b/src/win/udp.c @@ -404,11 +404,13 @@ static int uv__udp_send(uv_udp_send_t* req, uv_udp_t* handle, uv_buf_t bufs[], /* Request completed immediately. */ req->queued_bytes = 0; handle->reqs_pending++; + uv_ref(loop); uv_insert_pending_req(loop, (uv_req_t*)req); } else if (UV_SUCCEEDED_WITH_IOCP(result == 0)) { /* Request queued by the kernel. */ req->queued_bytes = uv_count_bufs(bufs, bufcnt); handle->reqs_pending++; + uv_ref(loop); } else { /* Send failed due to an error. */ uv__set_sys_error(loop, WSAGetLastError()); @@ -573,6 +575,7 @@ void uv_process_udp_send_req(uv_loop_t* loop, uv_udp_t* handle, } } + uv_unref(loop); DECREASE_PENDING_REQ_COUNT(handle); } From 5d21056277bf95a6703dfa107d35488cd8f960b2 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 9 Mar 2012 04:58:49 +0100 Subject: [PATCH 16/16] Windows: make sure that shutdown_cb is always called This patch changes how uv-win uses the UV_SHUTTING and UV_SHUT flags. UV_SHUT is now only used for tcp handles to track whether shutdown() has actually been called. UV_SHUTTING has the more generic meaning of "no longer readable". It would be good to replace it by an actual UV_READABLE flag in the future. This makes the shutdown_close_tcp and shutdown_close_pipe tests pass on windows. --- src/win/error.c | 2 ++ src/win/handle.c | 5 +++-- src/win/pipe.c | 25 +++++++++++++++++-------- src/win/stream.c | 2 ++ src/win/tcp.c | 10 +++++++--- src/win/tty.c | 18 ++++++++++++------ 6 files changed, 43 insertions(+), 19 deletions(-) diff --git a/src/win/error.c b/src/win/error.c index 3f6615f5..5d695696 100644 --- a/src/win/error.c +++ b/src/win/error.c @@ -91,6 +91,8 @@ uv_err_code uv_translate_sys_error(int sys_errno) { case WSAEFAULT: return UV_EFAULT; case ERROR_HOST_UNREACHABLE: return UV_EHOSTUNREACH; case WSAEHOSTUNREACH: return UV_EHOSTUNREACH; + case ERROR_OPERATION_ABORTED: return UV_EINTR; + case WSAEINTR: return UV_EINTR; case ERROR_INVALID_DATA: return UV_EINVAL; case WSAEINVAL: return UV_EINVAL; case ERROR_CANT_RESOLVE_FILENAME: return UV_ELOOP; diff --git a/src/win/handle.c b/src/win/handle.c index a4eadbdd..a1ff7275 100644 --- a/src/win/handle.c +++ b/src/win/handle.c @@ -91,9 +91,10 @@ void uv_close(uv_handle_t* handle, uv_close_cb cb) { tcp = (uv_tcp_t*)handle; /* If we don't shutdown before calling closesocket, windows will */ /* silently discard the kernel send buffer and reset the connection. */ - if (!(tcp->flags & UV_HANDLE_SHUT)) { + if ((tcp->flags & UV_HANDLE_CONNECTION) && + !(tcp->flags & UV_HANDLE_SHUT)) { shutdown(tcp->socket, SD_SEND); - tcp->flags |= UV_HANDLE_SHUT; + tcp->flags |= UV_HANDLE_SHUTTING | UV_HANDLE_SHUT; } tcp->flags &= ~(UV_HANDLE_READING | UV_HANDLE_LISTENING); closesocket(tcp->socket); diff --git a/src/win/pipe.c b/src/win/pipe.c index ab86018d..f99a32a9 100644 --- a/src/win/pipe.c +++ b/src/win/pipe.c @@ -281,11 +281,25 @@ void uv_pipe_endgame(uv_loop_t* loop, uv_pipe_t* handle) { IO_STATUS_BLOCK io_status; FILE_PIPE_LOCAL_INFORMATION pipe_info; - if (handle->flags & UV_HANDLE_SHUTTING && - !(handle->flags & UV_HANDLE_SHUT) && + if ((handle->flags & UV_HANDLE_CONNECTION) && + handle->shutdown_req != NULL && handle->write_reqs_pending == 0) { req = handle->shutdown_req; + /* Clear the shutdown_req field so we don't go here again. */ + handle->shutdown_req = NULL; + + if (handle->flags & UV_HANDLE_CLOSING) { + /* Already closing. Cancel the shutdown. */ + if (req->cb) { + uv__set_sys_error(loop, WSAEINTR); + req->cb(req, -1); + } + uv_unref(loop); + DECREASE_PENDING_REQ_COUNT(handle); + return; + } + /* Try to avoid flushing the pipe buffer in the thread pool. */ nt_status = pNtQueryInformationFile(handle->handle, &io_status, @@ -306,8 +320,6 @@ void uv_pipe_endgame(uv_loop_t* loop, uv_pipe_t* handle) { } if (pipe_info.OutboundQuota == pipe_info.WriteQuotaAvailable) { - handle->flags |= UV_HANDLE_SHUT; - /* Short-circuit, no need to call FlushFileBuffers. */ uv_insert_pending_req(loop, (uv_req_t*) req); return; @@ -318,8 +330,6 @@ void uv_pipe_endgame(uv_loop_t* loop, uv_pipe_t* handle) { req, WT_EXECUTELONGFUNCTION); if (result) { - /* Mark the handle as shut now to avoid going through this again. */ - handle->flags |= UV_HANDLE_SHUT; return; } else { @@ -625,6 +635,7 @@ void close_pipe(uv_pipe_t* handle, int* status, uv_err_t* err) { } if (handle->flags & UV_HANDLE_CONNECTION) { + handle->flags |= UV_HANDLE_SHUTTING; eof_timer_destroy(handle); } @@ -633,8 +644,6 @@ void close_pipe(uv_pipe_t* handle, int* status, uv_err_t* err) { CloseHandle(handle->handle); handle->handle = INVALID_HANDLE_VALUE; } - - handle->flags |= UV_HANDLE_SHUT; } diff --git a/src/win/stream.c b/src/win/stream.c index 53c4d9ec..d36bc682 100644 --- a/src/win/stream.c +++ b/src/win/stream.c @@ -47,6 +47,8 @@ void uv_connection_init(uv_stream_t* handle) { handle->read_req.wait_handle = INVALID_HANDLE_VALUE; handle->read_req.type = UV_READ; handle->read_req.data = handle; + + handle->shutdown_req = NULL; } diff --git a/src/win/tcp.c b/src/win/tcp.c index 31de8448..25ca26bd 100644 --- a/src/win/tcp.c +++ b/src/win/tcp.c @@ -171,11 +171,13 @@ void uv_tcp_endgame(uv_loop_t* loop, uv_tcp_t* handle) { uv_tcp_accept_t* req; if (handle->flags & UV_HANDLE_CONNECTION && - handle->flags & UV_HANDLE_SHUTTING && - !(handle->flags & UV_HANDLE_SHUT) && + handle->shutdown_req != NULL && handle->write_reqs_pending == 0) { - if (shutdown(handle->socket, SD_SEND) != SOCKET_ERROR) { + if (handle->flags & UV_HANDLE_CLOSING) { + status = -1; + sys_error = WSAEINTR; + } else if (shutdown(handle->socket, SD_SEND) != SOCKET_ERROR) { status = 0; handle->flags |= UV_HANDLE_SHUT; } else { @@ -189,6 +191,8 @@ void uv_tcp_endgame(uv_loop_t* loop, uv_tcp_t* handle) { handle->shutdown_req->cb(handle->shutdown_req, status); } + handle->shutdown_req = NULL; + uv_unref(loop); DECREASE_PENDING_REQ_COUNT(handle); return; diff --git a/src/win/tty.c b/src/win/tty.c index 2a696c89..601bf5ed 100644 --- a/src/win/tty.c +++ b/src/win/tty.c @@ -1721,6 +1721,8 @@ void uv_process_tty_write_req(uv_loop_t* loop, uv_tty_t* handle, void uv_tty_close(uv_tty_t* handle) { + handle->flags |= UV_HANDLE_SHUTTING; + uv_tty_read_stop(handle); CloseHandle(handle->handle); @@ -1731,17 +1733,21 @@ void uv_tty_close(uv_tty_t* handle) { void uv_tty_endgame(uv_loop_t* loop, uv_tty_t* handle) { - if (handle->flags & UV_HANDLE_CONNECTION && - handle->flags & UV_HANDLE_SHUTTING && - !(handle->flags & UV_HANDLE_SHUT) && + if ((handle->flags && UV_HANDLE_CONNECTION) && + handle->shutdown_req != NULL && handle->write_reqs_pending == 0) { - handle->flags |= UV_HANDLE_SHUT; - /* TTY shutdown is really just a no-op */ if (handle->shutdown_req->cb) { - handle->shutdown_req->cb(handle->shutdown_req, 0); + if (handle->flags & UV_HANDLE_CLOSING) { + uv__set_sys_error(loop, WSAEINTR); + handle->shutdown_req->cb(handle->shutdown_req, -1); + } else { + handle->shutdown_req->cb(handle->shutdown_req, 0); + } } + handle->shutdown_req = NULL; + uv_unref(loop); DECREASE_PENDING_REQ_COUNT(handle); return;