Gracefully handle uv_read_start() failures

Under specific rare timing circumstances the uv_read_start() could
fail with UV_EINVAL when the connection is reset between the connect (or
accept) and the uv_read_start() call on the nmworker loop.  Handle such
situation gracefully by propagating the errors from uv_read_start() into
upper layers, so the socket can be internally closed().

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 1a9114f8ae7a6010286c0788183284d2f17acec6..f1a6bce08eee2a5b2124006974b5345c773e20ed 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -2098,11 +2098,11 @@ isc__nm_tcpdns_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf);
 void
 isc__nm_tlsdns_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf);
 
-void
+isc_result_t
 isc__nm_start_reading(isc_nmsocket_t *sock);
 void
 isc__nm_stop_reading(isc_nmsocket_t *sock);
-void
+isc_result_t
 isc__nm_process_sock_buffer(isc_nmsocket_t *sock);
 void
 isc__nm_resume_processing(void *arg);

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index ad9dc3d57fdb193ab22987183a479ca228c7795b..6d73d0e28819884205bef6b939fe29f202944527 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -2161,39 +2161,42 @@ isc__nm_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf) {
        worker->recvbuf_inuse = true;
 }
 
-void
+isc_result_t
 isc__nm_start_reading(isc_nmsocket_t *sock) {
+       isc_result_t result = ISC_R_SUCCESS;
        int r;
 
        if (atomic_load(&sock->reading)) {
-               return;
+               return (ISC_R_SUCCESS);
        }
 
        switch (sock->type) {
        case isc_nm_udpsocket:
                r = uv_udp_recv_start(&sock->uv_handle.udp, isc__nm_alloc_cb,
                                      isc__nm_udp_read_cb);
-               UV_RUNTIME_CHECK(uv_udp_recv_start, r);
                break;
        case isc_nm_tcpsocket:
                r = uv_read_start(&sock->uv_handle.stream, isc__nm_alloc_cb,
                                  isc__nm_tcp_read_cb);
-               UV_RUNTIME_CHECK(uv_read_start, r);
                break;
        case isc_nm_tcpdnssocket:
                r = uv_read_start(&sock->uv_handle.stream, isc__nm_alloc_cb,
                                  isc__nm_tcpdns_read_cb);
-               UV_RUNTIME_CHECK(uv_read_start, r);
                break;
        case isc_nm_tlsdnssocket:
                r = uv_read_start(&sock->uv_handle.stream, isc__nm_alloc_cb,
                                  isc__nm_tlsdns_read_cb);
-               UV_RUNTIME_CHECK(uv_read_start, r);
                break;
        default:
                UNREACHABLE();
        }
-       atomic_store(&sock->reading, true);
+       if (r != 0) {
+               result = isc_uverr2result(r);
+       } else {
+               atomic_store(&sock->reading, true);
+       }
+
+       return (result);
 }
 
 void
@@ -2254,7 +2257,7 @@ processbuffer(isc_nmsocket_t *sock) {
  * Stop reading if this is a client socket.  In this case we'll be
  * called again later by isc__nm_resume_processing().
  */
-void
+isc_result_t
 isc__nm_process_sock_buffer(isc_nmsocket_t *sock) {
        for (;;) {
                int_fast32_t ah = atomic_load(&sock->ah);
@@ -2265,7 +2268,10 @@ isc__nm_process_sock_buffer(isc_nmsocket_t *sock) {
                         * Don't reset the timer until we have a
                         * full DNS message.
                         */
-                       isc__nm_start_reading(sock);
+                       result = isc__nm_start_reading(sock);
+                       if (result != ISC_R_SUCCESS) {
+                               return (result);
+                       }
                        /*
                         * Start the timer only if there are no externally used
                         * active handles, there's always one active handle
@@ -2275,11 +2281,11 @@ isc__nm_process_sock_buffer(isc_nmsocket_t *sock) {
                        if (ah == 1) {
                                isc__nmsocket_timer_start(sock);
                        }
-                       return;
+                       goto done;
                case ISC_R_CANCELED:
                        isc__nmsocket_timer_stop(sock);
                        isc__nm_stop_reading(sock);
-                       return;
+                       goto done;
                case ISC_R_SUCCESS:
                        /*
                         * Stop the timer on the successful message read, this
@@ -2290,13 +2296,15 @@ isc__nm_process_sock_buffer(isc_nmsocket_t *sock) {
 
                        if (atomic_load(&sock->client)) {
                                isc__nm_stop_reading(sock);
-                               return;
+                               goto done;
                        }
                        break;
                default:
                        UNREACHABLE();
                }
        }
+done:
+       return (ISC_R_SUCCESS);
 }
 
 void

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 86e7c85e2644c2000e76b48419fe768a180266e1..f7af009081bf6c242974143cb3a72ec0dc05cc2d 100644 (file)
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -747,18 +747,24 @@ isc__nm_async_tcpstartread(isc__networker_t *worker, isc__netievent_t *ev0) {
        isc__netievent_tcpstartread_t *ievent =
                (isc__netievent_tcpstartread_t *)ev0;
        isc_nmsocket_t *sock = ievent->sock;
+       isc_result_t result;
 
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(sock->tid == isc_nm_tid());
        UNUSED(worker);
 
        if (isc__nmsocket_closing(sock)) {
+               result = ISC_R_CANCELED;
+       } else {
+               result = isc__nm_start_reading(sock);
+       }
+
+       if (result != ISC_R_SUCCESS) {
                atomic_store(&sock->reading, true);
-               isc__nm_tcp_failed_read_cb(sock, ISC_R_CANCELED);
+               isc__nm_tcp_failed_read_cb(sock, result);
                return;
        }
 
-       isc__nm_start_reading(sock);
        isc__nmsocket_timer_start(sock);
 }
 

diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index e896d4ea4fb82f1d7ae939c449a0414fd0dc7ceb..e0da11b43f97b597c3ab9e5a39106a17fb5aaeb7 100644 (file)
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -714,6 +714,7 @@ isc__nm_async_tcpdnsread(isc__networker_t *worker, isc__netievent_t *ev0) {
        isc__netievent_tcpdnsread_t *ievent =
                (isc__netievent_tcpdnsread_t *)ev0;
        isc_nmsocket_t *sock = ievent->sock;
+       isc_result_t result;
 
        UNUSED(worker);
 
@@ -721,12 +722,15 @@ isc__nm_async_tcpdnsread(isc__networker_t *worker, isc__netievent_t *ev0) {
        REQUIRE(sock->tid == isc_nm_tid());
 
        if (isc__nmsocket_closing(sock)) {
-               atomic_store(&sock->reading, true);
-               isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false);
-               return;
+               result = ISC_R_CANCELED;
+       } else {
+               result = isc__nm_process_sock_buffer(sock);
        }
 
-       isc__nm_process_sock_buffer(sock);
+       if (result != ISC_R_SUCCESS) {
+               atomic_store(&sock->reading, true);
+               isc__nm_failed_read_cb(sock, result, false);
+       }
 }
 
 /*
@@ -836,6 +840,7 @@ isc__nm_tcpdns_read_cb(uv_stream_t *stream, ssize_t nread,
        isc_nmsocket_t *sock = uv_handle_get_data((uv_handle_t *)stream);
        uint8_t *base = NULL;
        size_t len;
+       isc_result_t result;
 
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(sock->tid == isc_nm_tid());
@@ -878,7 +883,10 @@ isc__nm_tcpdns_read_cb(uv_stream_t *stream, ssize_t nread,
                sock->read_timeout = atomic_load(&sock->mgr->idle);
        }
 
-       isc__nm_process_sock_buffer(sock);
+       result = isc__nm_process_sock_buffer(sock);
+       if (result != ISC_R_SUCCESS) {
+               isc__nm_failed_read_cb(sock, result, true);
+       }
 free:
        if (nread < 0) {
                /*
@@ -1029,7 +1037,12 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
         * prep_destroy()->tcpdns_close_direct().
         */
        isc_nmhandle_attach(handle, &csock->recv_handle);
-       isc__nm_process_sock_buffer(csock);
+       result = isc__nm_process_sock_buffer(csock);
+       if (result != ISC_R_SUCCESS) {
+               isc_nmhandle_detach(&csock->recv_handle);
+               isc_nmhandle_detach(&handle);
+               goto failure;
+       }
 
        /*
         * The initial timer has been set, update the read timeout for the next

diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c
index b3164496471736f1be3876782a359a96969450d5..74b66ac0a87e2fee9f57b826f9069adfee1537a0 100644 (file)
--- a/lib/isc/netmgr/tlsdns.c
+++ b/lib/isc/netmgr/tlsdns.c
@@ -316,7 +316,11 @@ tlsdns_connect_cb(uv_connect_t *uvreq, int status) {
        /* Setting pending req */
        sock->tls.pending_req = req;
 
-       isc__nm_process_sock_buffer(sock);
+       result = isc__nm_process_sock_buffer(sock);
+       if (result != ISC_R_SUCCESS) {
+               sock->tls.pending_req = NULL;
+               goto error;
+       }
 
        result = tls_cycle(sock);
        if (result != ISC_R_SUCCESS) {
@@ -1068,8 +1072,10 @@ tls_cycle_input(isc_nmsocket_t *sock) {
                                /*
                                 * Process what's in the buffer so far
                                 */
-                               isc__nm_process_sock_buffer(sock);
-
+                               result = isc__nm_process_sock_buffer(sock);
+                               if (result != ISC_R_SUCCESS) {
+                                       goto failure;
+                               }
                                /*
                                 * FIXME: Should we call
                                 * isc__nm_failed_read_cb()?
@@ -1081,7 +1087,10 @@ tls_cycle_input(isc_nmsocket_t *sock) {
 
                        sock->buf_len += len;
 
-                       isc__nm_process_sock_buffer(sock);
+                       result = isc__nm_process_sock_buffer(sock);
+                       if (result != ISC_R_SUCCESS) {
+                               goto failure;
+                       }
                }
        } else if (!SSL_is_init_finished(sock->tls.tls)) {
                if (SSL_is_server(sock->tls.tls)) {
@@ -1103,7 +1112,10 @@ tls_cycle_input(isc_nmsocket_t *sock) {
                if (sock->tls.state == TLS_STATE_NONE &&
                    !SSL_is_init_finished(sock->tls.tls)) {
                        sock->tls.state = TLS_STATE_HANDSHAKE;
-                       isc__nm_process_sock_buffer(sock);
+                       result = isc__nm_process_sock_buffer(sock);
+                       if (result != ISC_R_SUCCESS) {
+                               goto failure;
+                       }
                }
                /* else continue reading */
                break;
@@ -1656,7 +1668,10 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
 
        isc_nmhandle_detach(&handle);
 
-       isc__nm_process_sock_buffer(csock);
+       result = isc__nm_process_sock_buffer(csock);
+       if (result != ISC_R_SUCCESS) {
+               goto failure;
+       }
 
        /*
         * sock is now attached to the handle.

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 505cd0442b62c61da3d3952538dbfcf0f2e83191..37a3f0480a9f9030fa072d7a3951d2a71c79eac5 100644 (file)
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -1118,7 +1118,7 @@ void
 isc__nm_async_udpread(isc__networker_t *worker, isc__netievent_t *ev0) {
        isc__netievent_udpread_t *ievent = (isc__netievent_udpread_t *)ev0;
        isc_nmsocket_t *sock = ievent->sock;
-       isc_result_t result = ISC_R_SUCCESS;
+       isc_result_t result;
 
        UNUSED(worker);
 
@@ -1129,6 +1129,8 @@ isc__nm_async_udpread(isc__networker_t *worker, isc__netievent_t *ev0) {
                result = ISC_R_SHUTTINGDOWN;
        } else if (isc__nmsocket_closing(sock)) {
                result = ISC_R_CANCELED;
+       } else {
+               result = isc__nm_start_reading(sock);
        }
 
        if (result != ISC_R_SUCCESS) {
@@ -1137,7 +1139,6 @@ isc__nm_async_udpread(isc__networker_t *worker, isc__netievent_t *ev0) {
                return;
        }
 
-       isc__nm_start_reading(sock);
        isc__nmsocket_timer_start(sock);
 }