unix: return actual error from uv_try_write()

addaleax · addaleax · commit ee24ce900e57 · 2019-06-10T15:17:21.000+02:00
So far, for some (?) errors, `uv_try_write()` returns `EAGAIN` regardless of the actual error, so `ECONNRESET` and `EPIPE` errors can be swallowed here. This commit changes `uv_try_write()` so that it prefers to return the actual error it has seen. PR-URL: #2321 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -137,6 +137,7 @@ set(uv_test_sources
     test/test-tcp-read-stop.c
     test/test-tcp-shutdown-after-write.c
     test/test-tcp-try-write.c
+    test/test-tcp-try-write-error.c
     test/test-tcp-unexpected-read.c
     test/test-tcp-write-after-connect.c
     test/test-tcp-write-fail.c
diff --git a/Makefile.am b/Makefile.am
@@ -272,6 +272,7 @@ test_run_tests_SOURCES = test/blackhole-server.c \
                          test/test-tcp-writealot.c \
                          test/test-tcp-write-fail.c \
                          test/test-tcp-try-write.c \
+                         test/test-tcp-try-write-error.c \
                          test/test-tcp-write-queue-order.c \
                          test/test-thread-equal.c \
                          test/test-thread.c \
diff --git a/src/unix/stream.c b/src/unix/stream.c
@@ -1541,7 +1541,7 @@ int uv_try_write(uv_stream_t* stream,
   }
 
   if (written == 0 && req_size != 0)
-    return UV_EAGAIN;
+    return req.error < 0 ? req.error : UV_EAGAIN;
   else
     return written;
 }
diff --git a/test/test-list.h b/test/test-list.h
@@ -91,6 +91,7 @@ TEST_DECLARE   (tcp_write_after_connect)
 TEST_DECLARE   (tcp_writealot)
 TEST_DECLARE   (tcp_write_fail)
 TEST_DECLARE   (tcp_try_write)
+TEST_DECLARE   (tcp_try_write_error)
 TEST_DECLARE   (tcp_write_queue_order)
 TEST_DECLARE   (tcp_open)
 TEST_DECLARE   (tcp_open_twice)
@@ -582,6 +583,7 @@ TASK_LIST_START
   TEST_HELPER (tcp_write_fail, tcp4_echo_server)
 
   TEST_ENTRY  (tcp_try_write)
+  TEST_ENTRY  (tcp_try_write_error)
 
   TEST_ENTRY  (tcp_write_queue_order)
 
diff --git a/test/test-tcp-try-write-error.c b/test/test-tcp-try-write-error.c
@@ -0,0 +1,109 @@
+/* Copyright libuv 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"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+static uv_tcp_t server;
+static uv_tcp_t client;
+static uv_tcp_t incoming;
+static int connect_cb_called;
+static int close_cb_called;
+static int connection_cb_called;
+
+
+static void close_cb(uv_handle_t* handle) {
+  close_cb_called++;
+}
+
+static void incoming_close_cb(uv_handle_t* handle) {
+  uv_buf_t buf;
+  int r = 1;
+
+  close_cb_called++;
+
+  buf = uv_buf_init("meow", 4);
+  while (r > 0)
+    r = uv_try_write((uv_stream_t*) &client, &buf, 1);
+  fprintf(stderr, "uv_try_write error: %d %s\n", r, uv_strerror(r));
+  ASSERT(r == UV_EPIPE || r == UV_ECONNABORTED);
+  ASSERT(client.write_queue_size == 0);
+}
+
+
+static void connect_cb(uv_connect_t* req, int status) {
+  ASSERT(status == 0);
+  connect_cb_called++;
+}
+
+
+static void connection_cb(uv_stream_t* tcp, int status) {
+  ASSERT(status == 0);
+
+  ASSERT(0 == uv_tcp_init(tcp->loop, &incoming));
+  ASSERT(0 == uv_accept(tcp, (uv_stream_t*) &incoming));
+
+  connection_cb_called++;
+  uv_close((uv_handle_t*) &incoming, incoming_close_cb);
+  uv_close((uv_handle_t*) tcp, close_cb);
+}
+
+
+static void start_server(void) {
+  struct sockaddr_in addr;
+
+  ASSERT(0 == uv_ip4_addr("0.0.0.0", TEST_PORT, &addr));
+
+  ASSERT(0 == uv_tcp_init(uv_default_loop(), &server));
+  ASSERT(0 == uv_tcp_bind(&server, (struct sockaddr*) &addr, 0));
+  ASSERT(0 == uv_listen((uv_stream_t*) &server, 128, connection_cb));
+}
+
+
+TEST_IMPL(tcp_try_write_error) {
+  uv_connect_t connect_req;
+  struct sockaddr_in addr;
+
+  start_server();
+
+  ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &addr));
+
+  ASSERT(0 == uv_tcp_init(uv_default_loop(), &client));
+  ASSERT(0 == uv_tcp_connect(&connect_req,
+                             &client,
+                             (struct sockaddr*) &addr,
+                             connect_cb));
+
+  ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT));
+  uv_close((uv_handle_t*) &client, close_cb);
+  ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT));
+
+  ASSERT(connect_cb_called == 1);
+  ASSERT(close_cb_called == 3);
+  ASSERT(connection_cb_called == 1);
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}
diff --git a/test/test.gyp b/test/test.gyp
@@ -120,6 +120,7 @@
         'test-tcp-writealot.c',
         'test-tcp-write-fail.c',
         'test-tcp-try-write.c',
+        'test-tcp-try-write-error.c',
         'test-tcp-unexpected-read.c',
         'test-tcp-oob.c',
         'test-tcp-read-stop.c',

Original file line number	Diff line number	Diff line change
`@@ -1541,7 +1541,7 @@ int uv_try_write(uv_stream_t* stream,`
`1541`	`1541`	`}`
`1542`	`1542`
`1543`	`1543`	`if (written == 0 && req_size != 0)`
`1544`		`- return UV_EAGAIN;`
	`1544`	`+ return req.error < 0 ? req.error : UV_EAGAIN;`
`1545`	`1545`	`else`
`1546`	`1546`	`return written;`
`1547`	`1547`	`}`