unix,win: make on_alloc_cb failures more resilient

Treat both the base being NULL or the length being 0 as ENOBUFS.

PR-URL: https://github.com/libuv/libuv/pull/997
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Makefile.am

@@ -216,6 +216,7 @@ test_run_tests_SOURCES = test/blackhole-server.c \
                          test/test-socket-buffer-size.c \
                          test/test-spawn.c \
                          test/test-stdio-over-pipes.c \
+                         test/test-tcp-alloc-cb-fail.c \
                          test/test-tcp-bind-error.c \
                          test/test-tcp-bind6-error.c \
                          test/test-tcp-close-accept.c \
@@ -247,6 +248,7 @@ test_run_tests_SOURCES = test/blackhole-server.c \
                          test/test-timer.c \
                          test/test-tmpdir.c \
                          test/test-tty.c \
+                         test/test-udp-alloc-cb-fail.c \
                          test/test-udp-bind.c \
                          test/test-udp-create-socket-early.c \
                          test/test-udp-dgram-too-big.c \

src/unix/stream.c

@@ -1124,8 +1124,9 @@ static void uv__read(uv_stream_t* stream) {
       && (count-- > 0)) {
     assert(stream->alloc_cb != NULL);
 
+    buf = uv_buf_init(NULL, 0);
     stream->alloc_cb((uv_handle_t*)stream, 64 * 1024, &buf);
-    if (buf.len == 0) {
+    if (buf.base == NULL || buf.len == 0) {
       /* User indicates it can't or won't handle the read. */
       stream->read_cb(stream, UV_ENOBUFS, &buf);
       return;

src/unix/udp.c

@@ -165,8 +165,9 @@ static void uv__udp_recvmsg(uv_udp_t* handle) {
   h.msg_name = &peer;
 
   do {
+    buf = uv_buf_init(NULL, 0);
     handle->alloc_cb((uv_handle_t*) handle, 64 * 1024, &buf);
-    if (buf.len == 0) {
+    if (buf.base == NULL || buf.len == 0) {
       handle->recv_cb(handle, UV_ENOBUFS, &buf, NULL, 0);
       return;
     }

src/win/pipe.c

@@ -1634,8 +1634,9 @@ void uv_process_pipe_read_req(uv_loop_t* loop, uv_pipe_t* handle,
         }
       }
 
+      buf = uv_buf_init(NULL, 0);
       handle->alloc_cb((uv_handle_t*) handle, avail, &buf);
-      if (buf.len == 0) {
+      if (buf.base == NULL || buf.len == 0) {
         handle->read_cb((uv_stream_t*) handle, UV_ENOBUFS, &buf);
         break;
       }

src/win/tcp.c

@@ -496,8 +496,10 @@ static void uv_tcp_queue_read(uv_loop_t* loop, uv_tcp_t* handle) {
   */
   if (loop->active_tcp_streams < uv_active_tcp_streams_threshold) {
     handle->flags &= ~UV_HANDLE_ZERO_READ;
+    handle->tcp.conn.read_buffer = uv_buf_init(NULL, 0);
     handle->alloc_cb((uv_handle_t*) handle, 65536, &handle->tcp.conn.read_buffer);
-    if (handle->tcp.conn.read_buffer.len == 0) {
+    if (handle->tcp.conn.read_buffer.base == NULL ||
+        handle->tcp.conn.read_buffer.len == 0) {
       handle->read_cb((uv_stream_t*) handle, UV_ENOBUFS, &handle->tcp.conn.read_buffer);
       return;
     }
@@ -1004,8 +1006,9 @@ void uv_process_tcp_read_req(uv_loop_t* loop, uv_tcp_t* handle,
 
     /* Do nonblocking reads until the buffer is empty */
     while (handle->flags & UV_HANDLE_READING) {
+      buf = uv_buf_init(NULL, 0);
       handle->alloc_cb((uv_handle_t*) handle, 65536, &buf);
-      if (buf.len == 0) {
+      if (buf.base == NULL || buf.len == 0) {
         handle->read_cb((uv_stream_t*) handle, UV_ENOBUFS, &buf);
         break;
       }

src/win/tty.c

@@ -509,8 +509,10 @@ static void uv_tty_queue_read_line(uv_loop_t* loop, uv_tty_t* handle) {
   req = &handle->read_req;
   memset(&req->u.io.overlapped, 0, sizeof(req->u.io.overlapped));
 
+  handle->tty.rd.read_line_buffer = uv_buf_init(NULL, 0);
   handle->alloc_cb((uv_handle_t*) handle, 8192, &handle->tty.rd.read_line_buffer);
-  if (handle->tty.rd.read_line_buffer.len == 0) {
+  if (handle->tty.rd.read_line_buffer.base == NULL ||
+      handle->tty.rd.read_line_buffer.len == 0) {
     handle->read_cb((uv_stream_t*) handle,
                     UV_ENOBUFS,
                     &handle->tty.rd.read_line_buffer);
@@ -833,8 +835,9 @@ void uv_process_tty_read_raw_req(uv_loop_t* loop, uv_tty_t* handle,
       if (handle->tty.rd.last_key_offset < handle->tty.rd.last_key_len) {
         /* Allocate a buffer if needed */
         if (buf_used == 0) {
+          buf = uv_buf_init(NULL, 0);
           handle->alloc_cb((uv_handle_t*) handle, 1024, &buf);
-          if (buf.len == 0) {
+          if (buf.base == NULL || buf.len == 0) {
             handle->read_cb((uv_stream_t*) handle, UV_ENOBUFS, &buf);
             goto out;
           }

src/win/udp.c

@@ -289,8 +289,9 @@ static void uv_udp_queue_recv(uv_loop_t* loop, uv_udp_t* handle) {
   if (loop->active_udp_streams < uv_active_udp_streams_threshold) {
     handle->flags &= ~UV_HANDLE_ZERO_READ;
 
+    handle->recv_buffer = uv_buf_init(NULL, 0);
     handle->alloc_cb((uv_handle_t*) handle, 65536, &handle->recv_buffer);
-    if (handle->recv_buffer.len == 0) {
+    if (handle->recv_buffer.base == NULL || handle->recv_buffer.len == 0) {
       handle->recv_cb(handle, UV_ENOBUFS, &handle->recv_buffer, NULL, 0);
       return;
     }
@@ -506,8 +507,9 @@ void uv_process_udp_recv_req(uv_loop_t* loop, uv_udp_t* handle,
 
     /* Do a nonblocking receive */
     /* TODO: try to read multiple datagrams at once. FIONREAD maybe? */
+    buf = uv_buf_init(NULL, 0);
     handle->alloc_cb((uv_handle_t*) handle, 65536, &buf);
-    if (buf.len == 0) {
+    if (buf.base == NULL || buf.len == 0) {
       handle->recv_cb(handle, UV_ENOBUFS, &buf, NULL, 0);
       goto done;
     }

test/test-list.h

@@ -61,6 +61,7 @@ TEST_DECLARE   (ipc_send_recv_pipe_inprocess)
 TEST_DECLARE   (ipc_send_recv_tcp)
 TEST_DECLARE   (ipc_send_recv_tcp_inprocess)
 TEST_DECLARE   (ipc_tcp_connection)
+TEST_DECLARE   (tcp_alloc_cb_fail)
 TEST_DECLARE   (tcp_ping_pong)
 TEST_DECLARE   (tcp_ping_pong_v6)
 TEST_DECLARE   (pipe_ping_pong)
@@ -106,6 +107,7 @@ TEST_DECLARE   (tcp_bind6_error_addrnotavail)
 TEST_DECLARE   (tcp_bind6_error_fault)
 TEST_DECLARE   (tcp_bind6_error_inval)
 TEST_DECLARE   (tcp_bind6_localhost_ok)
+TEST_DECLARE   (udp_alloc_cb_fail)
 TEST_DECLARE   (udp_bind)
 TEST_DECLARE   (udp_bind_reuseaddr)
 TEST_DECLARE   (udp_create_early)
@@ -416,6 +418,8 @@ TASK_LIST_START
   TEST_ENTRY  (ipc_send_recv_tcp_inprocess)
   TEST_ENTRY  (ipc_tcp_connection)
 
+  TEST_ENTRY  (tcp_alloc_cb_fail)
+
   TEST_ENTRY  (tcp_ping_pong)
   TEST_HELPER (tcp_ping_pong, tcp4_echo_server)
 
@@ -483,6 +487,7 @@ TASK_LIST_START
   TEST_ENTRY  (tcp_bind6_error_inval)
   TEST_ENTRY  (tcp_bind6_localhost_ok)
 
+  TEST_ENTRY  (udp_alloc_cb_fail)
   TEST_ENTRY  (udp_bind)
   TEST_ENTRY  (udp_bind_reuseaddr)
   TEST_ENTRY  (udp_create_early)

test/test-tcp-alloc-cb-fail.c

@@ -0,0 +1,123 @@
+/* Copyright libuv project and 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 <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "uv.h"
+#include "task.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 uv_write_t write_req;
+
+static char hello[] = "HELLO!";
+
+
+static void close_cb(uv_handle_t* handle) {
+  close_cb_called++;
+}
+
+static void write_cb(uv_write_t* req, int status) {
+  ASSERT(status == 0);
+}
+
+static void conn_alloc_cb(uv_handle_t* handle, size_t size, uv_buf_t* buf) {
+  /* Do nothing, read_cb should be called with UV_ENOBUFS. */
+}
+
+static void conn_read_cb(uv_stream_t* stream,
+                         ssize_t nread,
+                         const uv_buf_t* buf) {
+  ASSERT(nread == UV_ENOBUFS);
+  ASSERT(buf->base == NULL);
+  ASSERT(buf->len == 0);
+
+  uv_close((uv_handle_t*) &incoming, close_cb);
+  uv_close((uv_handle_t*) &client, close_cb);
+  uv_close((uv_handle_t*) &server, close_cb);
+}
+
+static void connect_cb(uv_connect_t* req, int status) {
+  int r;
+  uv_buf_t buf;
+
+  ASSERT(status == 0);
+  connect_cb_called++;
+
+  buf = uv_buf_init(hello, sizeof(hello));
+  r = uv_write(&write_req, req->handle, &buf, 1, write_cb);
+  ASSERT(r == 0);
+}
+
+
+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));
+  ASSERT(0 == uv_read_start((uv_stream_t*) &incoming,
+                            conn_alloc_cb,
+                            conn_read_cb));
+
+  connection_cb_called++;
+}
+
+
+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_alloc_cb_fail) {
+  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));
+
+  ASSERT(connect_cb_called == 1);
+  ASSERT(connection_cb_called == 1);
+  ASSERT(close_cb_called == 3);
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}

test/test-tcp-write-queue-order.c

@@ -46,13 +46,13 @@ static void close_cb(uv_handle_t* handle) {
   close_cb_called++;
 }
 
-void timer_cb(uv_timer_t* handle) {
+static void timer_cb(uv_timer_t* handle) {
   uv_close((uv_handle_t*) &client, close_cb);
   uv_close((uv_handle_t*) &server, close_cb);
   uv_close((uv_handle_t*) &incoming, close_cb);
 }
 
-void write_cb(uv_write_t* req, int status) {
+static void write_cb(uv_write_t* req, int status) {
   if (status == 0)
     write_callbacks++;
   else if (status == UV_ECANCELED)

test/test-udp-alloc-cb-fail.c

@@ -0,0 +1,197 @@
+/* Copyright libuv project 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.
+ */
+
+#include "uv.h"
+#include "task.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#define CHECK_HANDLE(handle) \
+  ASSERT((uv_udp_t*)(handle) == &server || (uv_udp_t*)(handle) == &client)
+
+static uv_udp_t server;
+static uv_udp_t client;
+
+static int cl_send_cb_called;
+static int cl_recv_cb_called;
+
+static int sv_send_cb_called;
+static int sv_recv_cb_called;
+
+static int close_cb_called;
+
+
+static void sv_alloc_cb(uv_handle_t* handle,
+                        size_t suggested_size,
+                        uv_buf_t* buf) {
+  static char slab[65536];
+  CHECK_HANDLE(handle);
+  buf->base = slab;
+  buf->len = sizeof(slab);
+}
+
+
+static void cl_alloc_cb(uv_handle_t* handle,
+                        size_t suggested_size,
+                        uv_buf_t* buf) {
+  /* Do nothing, recv_cb should be called with UV_ENOBUFS. */
+}
+
+
+static void close_cb(uv_handle_t* handle) {
+  CHECK_HANDLE(handle);
+  ASSERT(1 == uv_is_closing(handle));
+  close_cb_called++;
+}
+
+
+static void cl_recv_cb(uv_udp_t* handle,
+                       ssize_t nread,
+                       const uv_buf_t* buf,
+                       const struct sockaddr* addr,
+                       unsigned flags) {
+  CHECK_HANDLE(handle);
+  ASSERT(flags == 0);
+  ASSERT(nread == UV_ENOBUFS);
+
+  cl_recv_cb_called++;
+
+  uv_close((uv_handle_t*) handle, close_cb);
+}
+
+
+static void cl_send_cb(uv_udp_send_t* req, int status) {
+  int r;
+
+  ASSERT(req != NULL);
+  ASSERT(status == 0);
+  CHECK_HANDLE(req->handle);
+
+  r = uv_udp_recv_start(req->handle, cl_alloc_cb, cl_recv_cb);
+  ASSERT(r == 0);
+
+  cl_send_cb_called++;
+}
+
+
+static void sv_send_cb(uv_udp_send_t* req, int status) {
+  ASSERT(req != NULL);
+  ASSERT(status == 0);
+  CHECK_HANDLE(req->handle);
+
+  uv_close((uv_handle_t*) req->handle, close_cb);
+  free(req);
+
+  sv_send_cb_called++;
+}
+
+
+static void sv_recv_cb(uv_udp_t* handle,
+                       ssize_t nread,
+                       const uv_buf_t* rcvbuf,
+                       const struct sockaddr* addr,
+                       unsigned flags) {
+  uv_udp_send_t* req;
+  uv_buf_t sndbuf;
+  int r;
+
+  if (nread < 0) {
+    ASSERT(0 && "unexpected error");
+  }
+
+  if (nread == 0) {
+    /* Returning unused buffer */
+    /* Don't count towards sv_recv_cb_called */
+    ASSERT(addr == NULL);
+    return;
+  }
+
+  CHECK_HANDLE(handle);
+  ASSERT(flags == 0);
+
+  ASSERT(addr != NULL);
+  ASSERT(nread == 4);
+  ASSERT(!memcmp("PING", rcvbuf->base, nread));
+
+  r = uv_udp_recv_stop(handle);
+  ASSERT(r == 0);
+
+  req = malloc(sizeof *req);
+  ASSERT(req != NULL);
+
+  sndbuf = uv_buf_init("PONG", 4);
+  r = uv_udp_send(req, handle, &sndbuf, 1, addr, sv_send_cb);
+  ASSERT(r == 0);
+
+  sv_recv_cb_called++;
+}
+
+
+TEST_IMPL(udp_alloc_cb_fail) {
+  struct sockaddr_in addr;
+  uv_udp_send_t req;
+  uv_buf_t buf;
+  int r;
+
+  ASSERT(0 == uv_ip4_addr("0.0.0.0", TEST_PORT, &addr));
+
+  r = uv_udp_init(uv_default_loop(), &server);
+  ASSERT(r == 0);
+
+  r = uv_udp_bind(&server, (const struct sockaddr*) &addr, 0);
+  ASSERT(r == 0);
+
+  r = uv_udp_recv_start(&server, sv_alloc_cb, sv_recv_cb);
+  ASSERT(r == 0);
+
+  ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &addr));
+
+  r = uv_udp_init(uv_default_loop(), &client);
+  ASSERT(r == 0);
+
+  buf = uv_buf_init("PING", 4);
+  r = uv_udp_send(&req,
+                  &client,
+                  &buf,
+                  1,
+                  (const struct sockaddr*) &addr,
+                  cl_send_cb);
+  ASSERT(r == 0);
+
+  ASSERT(close_cb_called == 0);
+  ASSERT(cl_send_cb_called == 0);
+  ASSERT(cl_recv_cb_called == 0);
+  ASSERT(sv_send_cb_called == 0);
+  ASSERT(sv_recv_cb_called == 0);
+
+  uv_run(uv_default_loop(), UV_RUN_DEFAULT);
+
+  ASSERT(cl_send_cb_called == 1);
+  ASSERT(cl_recv_cb_called == 1);
+  ASSERT(sv_send_cb_called == 1);
+  ASSERT(sv_recv_cb_called == 1);
+  ASSERT(close_cb_called == 2);
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}

uv.gyp

@@ -364,6 +364,7 @@
         'test/test-spawn.c',
         'test/test-fs-poll.c',
         'test/test-stdio-over-pipes.c',
+        'test/test-tcp-alloc-cb-fail.c',
         'test/test-tcp-bind-error.c',
         'test/test-tcp-bind6-error.c',
         'test/test-tcp-close.c',
@@ -398,6 +399,7 @@
         'test/test-timer-from-check.c',
         'test/test-timer.c',
         'test/test-tty.c',
+        'test/test-udp-alloc-cb-fail.c',
         'test/test-udp-bind.c',
         'test/test-udp-create-socket-early.c',
         'test/test-udp-dgram-too-big.c',