From 46f36e3df1a666620f6749427f15651cbc4b7001 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Mon, 24 Aug 2020 12:47:17 -0400 Subject: [PATCH] Revert "unix,stream: clear read/write states on close/eof" This reverts commit 12be29f185261b8a7e6ada596fd805919cb2b133. The commit in question was introducing failures in the Node.js test suite. Refs: https://github.com/libuv/libuv/issues/2943 Refs: https://github.com/libuv/libuv/pull/2967 Refs: https://github.com/libuv/libuv/pull/2409 PR-URL: https://github.com/libuv/libuv/pull/2968 Reviewed-By: Santiago Gimeno Reviewed-By: Ben Noordhuis --- CMakeLists.txt | 3 - Makefile.am | 3 - src/unix/stream.c | 3 - src/win/tcp.c | 2 - test/echo-server.c | 10 +- test/test-list.h | 11 -- ...-not-readable-nor-writable-on-read-error.c | 104 ------------------ test/test-not-readable-on-eof.c | 103 ----------------- test/test-not-writable-after-shutdown.c | 69 ------------ 9 files changed, 2 insertions(+), 306 deletions(-) delete mode 100644 test/test-not-readable-nor-writable-on-read-error.c delete mode 100644 test/test-not-readable-on-eof.c delete mode 100644 test/test-not-writable-after-shutdown.c diff --git a/CMakeLists.txt b/CMakeLists.txt index c0ebc80c..e9bf77f7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -424,9 +424,6 @@ if(LIBUV_BUILD_TESTS) test/test-metrics.c test/test-multiple-listen.c test/test-mutexes.c - test/test-not-readable-nor-writable-on-read-error.c - test/test-not-readable-on-eof.c - test/test-not-writable-after-shutdown.c test/test-osx-select.c test/test-pass-always.c test/test-ping-pong.c diff --git a/Makefile.am b/Makefile.am index 60eff6d1..46308eaa 100644 --- a/Makefile.am +++ b/Makefile.am @@ -206,9 +206,6 @@ test_run_tests_SOURCES = test/blackhole-server.c \ test/test-metrics.c \ test/test-multiple-listen.c \ test/test-mutexes.c \ - test/test-not-readable-nor-writable-on-read-error.c \ - test/test-not-readable-on-eof.c \ - test/test-not-writable-after-shutdown.c \ test/test-osx-select.c \ test/test-pass-always.c \ test/test-ping-pong.c \ diff --git a/src/unix/stream.c b/src/unix/stream.c index f86cf11e..8327f9cc 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -1001,7 +1001,6 @@ uv_handle_type uv__handle_type(int fd) { static void uv__stream_eof(uv_stream_t* stream, const uv_buf_t* buf) { stream->flags |= UV_HANDLE_READ_EOF; stream->flags &= ~UV_HANDLE_READING; - stream->flags &= ~UV_HANDLE_READABLE; uv__io_stop(stream->loop, &stream->io_watcher, POLLIN); if (!uv__io_active(&stream->io_watcher, POLLOUT)) uv__handle_stop(stream); @@ -1189,7 +1188,6 @@ static void uv__read(uv_stream_t* stream) { #endif } else { /* Error. User should call uv_close(). */ - stream->flags &= ~(UV_HANDLE_READABLE | UV_HANDLE_WRITABLE); stream->read_cb(stream, UV__ERR(errno), &buf); if (stream->flags & UV_HANDLE_READING) { stream->flags &= ~UV_HANDLE_READING; @@ -1278,7 +1276,6 @@ int uv_shutdown(uv_shutdown_t* req, uv_stream_t* stream, uv_shutdown_cb cb) { req->cb = cb; stream->shutdown_req = req; stream->flags |= UV_HANDLE_SHUTTING; - stream->flags &= ~UV_HANDLE_WRITABLE; uv__io_start(stream->loop, &stream->io_watcher, POLLOUT); uv__stream_osx_interrupt_select(stream); diff --git a/src/win/tcp.c b/src/win/tcp.c index ece52952..0dcaa97d 100644 --- a/src/win/tcp.c +++ b/src/win/tcp.c @@ -1015,7 +1015,6 @@ void uv_process_tcp_read_req(uv_loop_t* loop, uv_tcp_t* handle, */ err = WSAECONNRESET; } - handle->flags &= ~(UV_HANDLE_READABLE | UV_HANDLE_WRITABLE); handle->read_cb((uv_stream_t*)handle, uv_translate_sys_error(err), @@ -1097,7 +1096,6 @@ void uv_process_tcp_read_req(uv_loop_t* loop, uv_tcp_t* handle, * Unix. */ err = WSAECONNRESET; } - handle->flags &= ~(UV_HANDLE_READABLE | UV_HANDLE_WRITABLE); handle->read_cb((uv_stream_t*)handle, uv_translate_sys_error(err), diff --git a/test/echo-server.c b/test/echo-server.c index 76dfdc0e..c65142ff 100644 --- a/test/echo-server.c +++ b/test/echo-server.c @@ -69,6 +69,7 @@ static void after_shutdown(uv_shutdown_t* req, int status) { free(req); } + static void after_read(uv_stream_t* handle, ssize_t nread, const uv_buf_t* buf) { @@ -95,20 +96,13 @@ static void after_read(uv_stream_t* handle, /* * Scan for the letter Q which signals that we should quit the server. * If we get QS it means close the stream. - * If we get QSH it means disable linger before close the socket. */ if (!server_closed) { for (i = 0; i < nread; i++) { if (buf->base[i] == 'Q') { if (i + 1 < nread && buf->base[i + 1] == 'S') { - int reset = 0; - if (i + 2 < nread && buf->base[i + 2] == 'H') - reset = 1; free(buf->base); - if (reset && handle->type == UV_TCP) - ASSERT(0 == uv_tcp_close_reset((uv_tcp_t*) handle, on_close)); - else - uv_close((uv_handle_t*) handle, on_close); + uv_close((uv_handle_t*)handle, on_close); return; } else { uv_close(server, on_server_close); diff --git a/test/test-list.h b/test/test-list.h index 310db149..52b17a69 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -501,10 +501,6 @@ TEST_DECLARE (handle_type_name) TEST_DECLARE (req_type_name) TEST_DECLARE (getters_setters) -TEST_DECLARE (not_writable_after_shutdown) -TEST_DECLARE (not_readable_nor_writable_on_read_error) -TEST_DECLARE (not_readable_on_eof) - #ifndef _WIN32 TEST_DECLARE (fork_timer) TEST_DECLARE (fork_socketpair) @@ -1119,13 +1115,6 @@ TASK_LIST_START TEST_ENTRY (idna_toascii) #endif - TEST_ENTRY (not_writable_after_shutdown) - TEST_HELPER (not_writable_after_shutdown, tcp4_echo_server) - TEST_ENTRY (not_readable_nor_writable_on_read_error) - TEST_HELPER (not_readable_nor_writable_on_read_error, tcp4_echo_server) - TEST_ENTRY (not_readable_on_eof) - TEST_HELPER (not_readable_on_eof, tcp4_echo_server) - TEST_ENTRY (metrics_idle_time) TEST_ENTRY (metrics_idle_time_thread) TEST_ENTRY (metrics_idle_time_zero) diff --git a/test/test-not-readable-nor-writable-on-read-error.c b/test/test-not-readable-nor-writable-on-read-error.c deleted file mode 100644 index ae951e39..00000000 --- a/test/test-not-readable-nor-writable-on-read-error.c +++ /dev/null @@ -1,104 +0,0 @@ -/* Copyright the libuv project 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. - */ - -#include "uv.h" -#include "task.h" - -static uv_loop_t loop; -static uv_tcp_t tcp_client; -static uv_connect_t connect_req; -static uv_write_t write_req; -static char reset_me_cmd[] = {'Q', 'S', 'H'}; - -static int connect_cb_called; -static int read_cb_called; -static int write_cb_called; -static int close_cb_called; - -static void write_cb(uv_write_t* req, int status) { - write_cb_called++; - ASSERT(status == 0); -} - -static void alloc_cb(uv_handle_t* handle, - size_t suggested_size, - uv_buf_t* buf) { - static char slab[64]; - buf->base = slab; - buf->len = sizeof(slab); -} - -static void close_cb(uv_handle_t* handle) { - close_cb_called++; -} - -static void read_cb(uv_stream_t* handle, ssize_t nread, const uv_buf_t* buf) { - read_cb_called++; - - ASSERT((nread < 0) && (nread != UV_EOF)); - ASSERT(0 == uv_is_writable(handle)); - ASSERT(0 == uv_is_readable(handle)); - - uv_close((uv_handle_t*) handle, close_cb); -} - -static void connect_cb(uv_connect_t* req, int status) { - int r; - uv_buf_t reset_me; - - connect_cb_called++; - ASSERT(status == 0); - - r = uv_read_start((uv_stream_t*) &tcp_client, alloc_cb, read_cb); - ASSERT(r == 0); - - reset_me = uv_buf_init(reset_me_cmd, sizeof(reset_me_cmd)); - - r = uv_write(&write_req, - (uv_stream_t*) &tcp_client, - &reset_me, - 1, - write_cb); - - ASSERT(r == 0); -} - -TEST_IMPL(not_readable_nor_writable_on_read_error) { - struct sockaddr_in sa; - ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &sa)); - ASSERT(0 == uv_loop_init(&loop)); - ASSERT(0 == uv_tcp_init(&loop, &tcp_client)); - - ASSERT(0 == uv_tcp_connect(&connect_req, - &tcp_client, - (const struct sockaddr*) &sa, - connect_cb)); - - ASSERT(0 == uv_run(&loop, UV_RUN_DEFAULT)); - - ASSERT(connect_cb_called == 1); - ASSERT(read_cb_called == 1); - ASSERT(write_cb_called == 1); - ASSERT(close_cb_called == 1); - - MAKE_VALGRIND_HAPPY(); - return 0; -} diff --git a/test/test-not-readable-on-eof.c b/test/test-not-readable-on-eof.c deleted file mode 100644 index 2bb5f4ee..00000000 --- a/test/test-not-readable-on-eof.c +++ /dev/null @@ -1,103 +0,0 @@ -/* Copyright the libuv project 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. - */ - -#include "uv.h" -#include "task.h" - -static uv_loop_t loop; -static uv_tcp_t tcp_client; -static uv_connect_t connect_req; -static uv_write_t write_req; -static char close_me_cmd[] = {'Q', 'S'}; - -static int connect_cb_called; -static int read_cb_called; -static int write_cb_called; -static int close_cb_called; - -static void write_cb(uv_write_t* req, int status) { - write_cb_called++; - ASSERT(status == 0); -} - -static void alloc_cb(uv_handle_t* handle, - size_t suggested_size, - uv_buf_t* buf) { - static char slab[64]; - buf->base = slab; - buf->len = sizeof(slab); -} - -static void close_cb(uv_handle_t* handle) { - close_cb_called++; -} - -static void read_cb(uv_stream_t* handle, ssize_t nread, const uv_buf_t* buf) { - read_cb_called++; - - ASSERT(nread == UV_EOF); - ASSERT(0 == uv_is_readable(handle)); - - uv_close((uv_handle_t*) handle, close_cb); -} - -static void connect_cb(uv_connect_t* req, int status) { - int r; - uv_buf_t close_me; - - connect_cb_called++; - ASSERT(status == 0); - - r = uv_read_start((uv_stream_t*) &tcp_client, alloc_cb, read_cb); - ASSERT(r == 0); - - close_me = uv_buf_init(close_me_cmd, sizeof(close_me_cmd)); - - r = uv_write(&write_req, - (uv_stream_t*) &tcp_client, - &close_me, - 1, - write_cb); - - ASSERT(r == 0); -} - -TEST_IMPL(not_readable_on_eof) { - struct sockaddr_in sa; - ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &sa)); - ASSERT(0 == uv_loop_init(&loop)); - ASSERT(0 == uv_tcp_init(&loop, &tcp_client)); - - ASSERT(0 == uv_tcp_connect(&connect_req, - &tcp_client, - (const struct sockaddr*) &sa, - connect_cb)); - - ASSERT(0 == uv_run(&loop, UV_RUN_DEFAULT)); - - ASSERT(connect_cb_called == 1); - ASSERT(read_cb_called == 1); - ASSERT(write_cb_called == 1); - ASSERT(close_cb_called == 1); - - MAKE_VALGRIND_HAPPY(); - return 0; -} diff --git a/test/test-not-writable-after-shutdown.c b/test/test-not-writable-after-shutdown.c deleted file mode 100644 index 9cd93703..00000000 --- a/test/test-not-writable-after-shutdown.c +++ /dev/null @@ -1,69 +0,0 @@ -/* Copyright the libuv project 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. - */ - -#include "uv.h" -#include "task.h" - -static uv_shutdown_t shutdown_req; - -static void close_cb(uv_handle_t* handle) { - -} - -static void shutdown_cb(uv_shutdown_t* req, int status) { - uv_close((uv_handle_t*) req->handle, close_cb); -} - -static void connect_cb(uv_connect_t* req, int status) { - int r; - ASSERT(status == 0); - - r = uv_shutdown(&shutdown_req, req->handle, shutdown_cb); - ASSERT(r == 0); - - ASSERT(0 == uv_is_writable(req->handle)); -} - -TEST_IMPL(not_writable_after_shutdown) { - int r; - struct sockaddr_in addr; - uv_loop_t* loop; - uv_tcp_t socket; - uv_connect_t connect_req; - - ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &addr)); - loop = uv_default_loop(); - - r = uv_tcp_init(loop, &socket); - ASSERT(r == 0); - - r = uv_tcp_connect(&connect_req, - &socket, - (const struct sockaddr*) &addr, - connect_cb); - ASSERT(r == 0); - - r = uv_run(uv_default_loop(), UV_RUN_DEFAULT); - ASSERT(r == 0); - - MAKE_VALGRIND_HAPPY(); - return 0; -}