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 <vtjnash@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>

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

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.
    */

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;
 

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,

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:

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