From 06b731742208c0c7b0b56948da2f2e0c9e645ec9 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 8 Jun 2020 09:03:29 +0200 Subject: [PATCH] unix,win: harmonize uv_read_start() error handling The behavior of `uv_read_start()` when the handle is closing or already busy reading wasn't consistent across platforms. Now it is. Fixes: https://github.com/libuv/help/issues/137 PR-URL: https://github.com/libuv/libuv/pull/2795 Reviewed-By: Jameson Nash Reviewed-By: Santiago Gimeno Reviewed-By: Colin Ihrig Reviewed-By: Bartosz Sosnowski --- docs/src/stream.rst | 5 +++++ src/unix/stream.c | 12 +++--------- src/uv-common.c | 16 ++++++++++++++++ src/uv-common.h | 4 ++++ src/win/stream.c | 13 +++---------- test/test-shutdown-eof.c | 5 ++++- 6 files changed, 35 insertions(+), 20 deletions(-) diff --git a/docs/src/stream.rst b/docs/src/stream.rst index 2ccb59b5..429ebdab 100644 --- a/docs/src/stream.rst +++ b/docs/src/stream.rst @@ -139,6 +139,11 @@ API be made several times until there is no more data to read or :c:func:`uv_read_stop` is called. + .. versionchanged:: 1.38.0 :c:func:`uv_read_start()` now consistently + returns `UV_EALREADY` when called twice, and `UV_EINVAL` when the + stream is closing. With older libuv versions, it returns `UV_EALREADY` + on Windows but not UNIX, and `UV_EINVAL` on UNIX but not Windows. + .. c:function:: int uv_read_stop(uv_stream_t*) Stop reading data from the stream. The :c:type:`uv_read_cb` callback will diff --git a/src/unix/stream.c b/src/unix/stream.c index 8327f9cc..106785e4 100644 --- a/src/unix/stream.c +++ b/src/unix/stream.c @@ -1552,18 +1552,12 @@ int uv_try_write(uv_stream_t* stream, } -int uv_read_start(uv_stream_t* stream, - uv_alloc_cb alloc_cb, - uv_read_cb read_cb) { +int uv__read_start(uv_stream_t* stream, + uv_alloc_cb alloc_cb, + uv_read_cb read_cb) { assert(stream->type == UV_TCP || stream->type == UV_NAMED_PIPE || stream->type == UV_TTY); - if (stream->flags & UV_HANDLE_CLOSING) - return UV_EINVAL; - - if (!(stream->flags & UV_HANDLE_READABLE)) - return UV_ENOTCONN; - /* The UV_HANDLE_READING flag is irrelevant of the state of the tcp - it just * expresses the desired state of the user. */ diff --git a/src/uv-common.c b/src/uv-common.c index 602e5f49..a314b194 100644 --- a/src/uv-common.c +++ b/src/uv-common.c @@ -832,6 +832,22 @@ void uv_loop_delete(uv_loop_t* loop) { } +int uv_read_start(uv_stream_t* stream, + uv_alloc_cb alloc_cb, + uv_read_cb read_cb) { + if (stream->flags & UV_HANDLE_CLOSING) + return UV_EINVAL; + + if (stream->flags & UV_HANDLE_READING) + return UV_EALREADY; + + if (!(stream->flags & UV_HANDLE_READABLE)) + return UV_ENOTCONN; + + return uv__read_start(stream, alloc_cb, read_cb); +} + + void uv_os_free_environ(uv_env_item_t* envitems, int count) { int i; diff --git a/src/uv-common.h b/src/uv-common.h index e851291c..a92912fd 100644 --- a/src/uv-common.h +++ b/src/uv-common.h @@ -136,6 +136,10 @@ int uv__loop_configure(uv_loop_t* loop, uv_loop_option option, va_list ap); void uv__loop_close(uv_loop_t* loop); +int uv__read_start(uv_stream_t* stream, + uv_alloc_cb alloc_cb, + uv_read_cb read_cb); + int uv__tcp_bind(uv_tcp_t* tcp, const struct sockaddr* addr, unsigned int addrlen, diff --git a/src/win/stream.c b/src/win/stream.c index 46a0709a..ebb5fe5c 100644 --- a/src/win/stream.c +++ b/src/win/stream.c @@ -65,18 +65,11 @@ int uv_accept(uv_stream_t* server, uv_stream_t* client) { } -int uv_read_start(uv_stream_t* handle, uv_alloc_cb alloc_cb, - uv_read_cb read_cb) { +int uv__read_start(uv_stream_t* handle, + uv_alloc_cb alloc_cb, + uv_read_cb read_cb) { int err; - if (handle->flags & UV_HANDLE_READING) { - return UV_EALREADY; - } - - if (!(handle->flags & UV_HANDLE_READABLE)) { - return UV_ENOTCONN; - } - err = ERROR_INVALID_PARAMETER; switch (handle->type) { case UV_TCP: diff --git a/test/test-shutdown-eof.c b/test/test-shutdown-eof.c index 9f95e756..a677ede9 100644 --- a/test/test-shutdown-eof.c +++ b/test/test-shutdown-eof.c @@ -89,7 +89,10 @@ static void connect_cb(uv_connect_t *req, int status) { ASSERT(req == &connect_req); /* Start reading from our connection so we can receive the EOF. */ - uv_read_start((uv_stream_t*)&tcp, alloc_cb, read_cb); + ASSERT_EQ(0, uv_read_start((uv_stream_t*)&tcp, alloc_cb, read_cb)); + + /* Check error handling. */ + ASSERT_EQ(UV_EALREADY, uv_read_start((uv_stream_t*)&tcp, alloc_cb, read_cb)); /* * Write the letter 'Q' to gracefully kill the echo-server. This will not