]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Fix the isc_nm_closedown() to actually close the pending connections
authorOndřej Surý <ondrej@sury.org>
Wed, 21 Oct 2020 10:52:09 +0000 (12:52 +0200)
committerEvan Hunt <each@isc.org>
Thu, 22 Oct 2020 22:00:00 +0000 (15:00 -0700)
1. The isc__nm_tcp_send() and isc__nm_tcp_read() was not checking
   whether the socket was still alive and scheduling reads/sends on
   closed socket.

2. The isc_nm_read(), isc_nm_send() and isc_nm_resumeread() have been
   changed to always return the error conditions via the callbacks, so
   they always succeed.  This applies to all protocols (UDP, TCP and
   TCPDNS).

(cherry picked from commit f7c82e406e83f98d3a1fecedc78725b441552092)

lib/isc/include/isc/netmgr.h
lib/isc/netmgr/netmgr-int.h
lib/isc/netmgr/netmgr.c
lib/isc/netmgr/tcp.c
lib/isc/netmgr/tcpdns.c
lib/isc/netmgr/udp.c
lib/ns/client.c
lib/ns/xfrout.c

index dc4f977e4201953f8211a8fc517fc0b91502416c..95822d1c7831df38f1d0c54ca7cd0ab5c6dc9a52 100644 (file)
@@ -200,7 +200,7 @@ isc_nm_resume(isc_nm_t *mgr);
  * workers to resume.
  */
 
-isc_result_t
+void
 isc_nm_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg);
 /*
  * Begin (or continue) reading on the socket associated with 'handle', and
@@ -228,7 +228,7 @@ isc_nm_cancelread(isc_nmhandle_t *handle);
  * \li ...for which a read/recv callback has been defined.
  */
 
-isc_result_t
+void
 isc_nm_resumeread(isc_nmhandle_t *handle);
 /*%<
  * Resume reading on the handle's socket.
@@ -238,7 +238,7 @@ isc_nm_resumeread(isc_nmhandle_t *handle);
  * \li ...for a socket with a defined read/recv callback.
  */
 
-isc_result_t
+void
 isc_nm_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
            void *cbarg);
 /*%<
index b9a9672783891284c13ed2751c6041b33bf3a47b..dd1366970abff7b8aea554a7a205db540d78bed2 100644 (file)
@@ -679,7 +679,7 @@ isc__nm_async_shutdown(isc__networker_t *worker, isc__netievent_t *ev0);
  * close on them.
  */
 
-isc_result_t
+void
 isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
                 void *cbarg);
 /*%<
@@ -700,14 +700,14 @@ isc__nm_async_udpsend(isc__networker_t *worker, isc__netievent_t *ev0);
  * Callback handlers for asynchronous UDP events (listen, stoplisten, send).
  */
 
-isc_result_t
+void
 isc__nm_tcp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
                 void *cbarg);
 /*%<
  * Back-end implementation of isc_nm_send() for TCP handles.
  */
 
-isc_result_t
+void
 isc__nm_tcp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg);
 /*
  * Back-end implementation of isc_nm_read() for TCP handles.
@@ -724,7 +724,7 @@ isc__nm_tcp_pauseread(isc_nmsocket_t *sock);
  * Pause reading on this socket, while still remembering the callback.
  */
 
-isc_result_t
+void
 isc__nm_tcp_resumeread(isc_nmsocket_t *sock);
 /*%<
  * Resume reading from socket.
@@ -774,7 +774,7 @@ isc__nm_async_tcpclose(isc__networker_t *worker, isc__netievent_t *ev0);
  * stoplisten, send, read, pause, close).
  */
 
-isc_result_t
+void
 isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
                    isc_nm_cb_t cb, void *cbarg);
 /*%<
index e521f752c4fff5e0691e4f31124ac37f0a7c6427..bdcfd6653190d6f521986b72c20459bcc8f9344a 100644 (file)
@@ -429,6 +429,9 @@ isc_nm_destroy(isc_nm_t **mgr0) {
 #endif /* ifdef WIN32 */
        }
 
+#ifdef NETMGR_TRACE
+       isc__nm_dump_active(mgr);
+#endif
        INSIST(references == 1);
 
        /*
@@ -1428,7 +1431,7 @@ isc__nm_uvreq_put(isc__nm_uvreq_t **req0, isc_nmsocket_t *sock) {
        isc__nmsocket_detach(&sock);
 }
 
-isc_result_t
+void
 isc_nm_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
            void *cbarg) {
        REQUIRE(VALID_NMHANDLE(handle));
@@ -1436,24 +1439,28 @@ isc_nm_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
        switch (handle->sock->type) {
        case isc_nm_udpsocket:
        case isc_nm_udplistener:
-               return (isc__nm_udp_send(handle, region, cb, cbarg));
+               isc__nm_udp_send(handle, region, cb, cbarg);
+               break;
        case isc_nm_tcpsocket:
-               return (isc__nm_tcp_send(handle, region, cb, cbarg));
+               isc__nm_tcp_send(handle, region, cb, cbarg);
+               break;
        case isc_nm_tcpdnssocket:
-               return (isc__nm_tcpdns_send(handle, region, cb, cbarg));
+               isc__nm_tcpdns_send(handle, region, cb, cbarg);
+               break;
        default:
                INSIST(0);
                ISC_UNREACHABLE();
        }
 }
 
-isc_result_t
+void
 isc_nm_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
        REQUIRE(VALID_NMHANDLE(handle));
 
        switch (handle->sock->type) {
        case isc_nm_tcpsocket:
-               return (isc__nm_tcp_read(handle, cb, cbarg));
+               isc__nm_tcp_read(handle, cb, cbarg);
+               break;
        default:
                INSIST(0);
                ISC_UNREACHABLE();
@@ -1489,7 +1496,7 @@ isc_nm_pauseread(isc_nmhandle_t *handle) {
        }
 }
 
-isc_result_t
+void
 isc_nm_resumeread(isc_nmhandle_t *handle) {
        REQUIRE(VALID_NMHANDLE(handle));
 
@@ -1497,7 +1504,8 @@ isc_nm_resumeread(isc_nmhandle_t *handle) {
 
        switch (sock->type) {
        case isc_nm_tcpsocket:
-               return (isc__nm_tcp_resumeread(sock));
+               isc__nm_tcp_resumeread(sock);
+               break;
        default:
                INSIST(0);
                ISC_UNREACHABLE();
@@ -1838,7 +1846,8 @@ nmhandle_dump(isc_nmhandle_t *handle) {
 
 static void
 nmsocket_dump(isc_nmsocket_t *sock) {
-       isc_nmhandle_t *handle;
+       isc_nmhandle_t *handle = NULL;
+
        LOCK(&sock->lock);
        fprintf(stderr, "\n=================\n");
        fprintf(stderr, "Active socket %p, type %s, refs %lu\n", sock,
@@ -1850,25 +1859,37 @@ nmsocket_dump(isc_nmsocket_t *sock) {
        backtrace_symbols_fd(sock->backtrace, sock->backtrace_size,
                             STDERR_FILENO);
        fprintf(stderr, "\n");
-       fprintf(stderr, "Active handles:\n");
+
        for (handle = ISC_LIST_HEAD(sock->active_handles); handle != NULL;
             handle = ISC_LIST_NEXT(handle, active_link))
        {
+               static bool first = true;
+               if (first) {
+                       fprintf(stderr, "Active handles:\n");
+                       first = false;
+               }
                nmhandle_dump(handle);
        }
+
        fprintf(stderr, "\n");
        UNLOCK(&sock->lock);
 }
 
 void
 isc__nm_dump_active(isc_nm_t *nm) {
-       isc_nmsocket_t *sock;
+       isc_nmsocket_t *sock = NULL;
+
        REQUIRE(VALID_NM(nm));
+
        LOCK(&nm->lock);
-       fprintf(stderr, "Outstanding sockets\n");
        for (sock = ISC_LIST_HEAD(nm->active_sockets); sock != NULL;
             sock = ISC_LIST_NEXT(sock, active_link))
        {
+               static bool first = true;
+               if (first) {
+                       fprintf(stderr, "Outstanding sockets\n");
+                       first = false;
+               }
                nmsocket_dump(sock);
        }
        UNLOCK(&nm->lock);
index ecfef2c96d7a5042bb21e9b9cf7577162b480297..6cdd9ddadfabb8e86a151663bf610646ad0e871c 100644 (file)
@@ -583,11 +583,33 @@ tcp_listenclose_cb(uv_handle_t *handle) {
        isc__nmsocket_detach(&sock);
 }
 
+static void
+failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) {
+       isc_nm_recv_cb_t cb;
+       void *cbarg = NULL;
+
+       uv_read_stop(&sock->uv_handle.stream);
+
+       if (sock->timer_initialized) {
+               uv_timer_stop(&sock->timer);
+       }
+
+       if (sock->quota) {
+               isc_quota_detach(&sock->quota);
+       }
+
+       cb = sock->recv_cb;
+       cbarg = sock->recv_cbarg;
+       isc__nmsocket_clearcb(sock);
+
+       if (cb != NULL) {
+               cb(sock->statichandle, result, NULL, cbarg);
+       }
+}
+
 static void
 readtimeout_cb(uv_timer_t *handle) {
        isc_nmsocket_t *sock = uv_handle_get_data((uv_handle_t *)handle);
-       isc_nm_recv_cb_t cb;
-       void *cbarg;
 
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(sock->tid == isc_nm_tid());
@@ -604,29 +626,22 @@ readtimeout_cb(uv_timer_t *handle) {
        /*
         * Timeout; stop reading and process whatever we have.
         */
-       uv_read_stop(&sock->uv_handle.stream);
-       if (sock->quota) {
-               isc_quota_detach(&sock->quota);
-       }
-
-       cb = sock->recv_cb;
-       cbarg = sock->recv_cbarg;
-       isc__nmsocket_clearcb(sock);
-
-       if (cb != NULL) {
-               cb(sock->statichandle, ISC_R_TIMEDOUT, NULL, cbarg);
-       }
+       failed_read_cb(sock, ISC_R_TIMEDOUT);
 }
 
-isc_result_t
+void
 isc__nm_tcp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
-       isc_nmsocket_t *sock = NULL;
+       isc_nmsocket_t *sock = handle->sock;
        isc__netievent_startread_t *ievent = NULL;
 
        REQUIRE(VALID_NMHANDLE(handle));
        REQUIRE(VALID_NMSOCK(handle->sock));
 
-       sock = handle->sock;
+       if (!isc__nmsocket_active(sock)) {
+               isc__nm_incstats(sock->mgr, sock->statsindex[STATID_RECVFAIL]);
+               cb(handle, ISC_R_CANCELED, NULL, cbarg);
+               return;
+       }
 
        REQUIRE(sock->tid == isc_nm_tid());
        sock->recv_cb = cb;
@@ -644,7 +659,7 @@ isc__nm_tcp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
                                       (isc__netievent_t *)ievent);
        }
 
-       return (ISC_R_SUCCESS);
+       return;
 }
 
 /*%<
@@ -678,6 +693,16 @@ isc__nm_async_tcp_startread(isc__networker_t *worker, isc__netievent_t *ev0) {
        int r;
 
        REQUIRE(worker->id == isc_nm_tid());
+
+       r = uv_read_start(&sock->uv_handle.stream, tcp_alloc_cb, read_cb);
+       if (r != 0) {
+               isc__nm_incstats(sock->mgr, sock->statsindex[STATID_RECVFAIL]);
+
+               failed_read_cb(sock, ISC_R_CANCELED);
+
+               return;
+       }
+
        if (sock->read_timeout != 0) {
                if (!sock->timer_initialized) {
                        uv_timer_init(&worker->loop, &sock->timer);
@@ -687,11 +712,6 @@ isc__nm_async_tcp_startread(isc__networker_t *worker, isc__netievent_t *ev0) {
                uv_timer_start(&sock->timer, readtimeout_cb, sock->read_timeout,
                               0);
        }
-
-       r = uv_read_start(&sock->uv_handle.stream, tcp_alloc_cb, read_cb);
-       if (r != 0) {
-               isc__nm_incstats(sock->mgr, sock->statsindex[STATID_RECVFAIL]);
-       }
 }
 
 isc_result_t
@@ -734,7 +754,7 @@ isc__nm_async_tcp_pauseread(isc__networker_t *worker, isc__netievent_t *ev0) {
        uv_read_stop(&sock->uv_handle.stream);
 }
 
-isc_result_t
+void
 isc__nm_tcp_resumeread(isc_nmsocket_t *sock) {
        isc__netievent_startread_t *ievent = NULL;
 
@@ -742,11 +762,16 @@ isc__nm_tcp_resumeread(isc_nmsocket_t *sock) {
        REQUIRE(sock->tid == isc_nm_tid());
 
        if (sock->recv_cb == NULL) {
-               return (ISC_R_CANCELED);
+               return;
+       }
+
+       if (!isc__nmsocket_active(sock)) {
+               failed_read_cb(sock, ISC_R_CANCELED);
+               return;
        }
 
        if (!atomic_load(&sock->readpaused)) {
-               return (ISC_R_SUCCESS);
+               return;
        }
 
        atomic_store(&sock->readpaused, false);
@@ -762,26 +787,21 @@ isc__nm_tcp_resumeread(isc_nmsocket_t *sock) {
                isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
                                       (isc__netievent_t *)ievent);
        }
-
-       return (ISC_R_SUCCESS);
 }
 
 static void
 read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
        isc_nmsocket_t *sock = uv_handle_get_data((uv_handle_t *)stream);
-       isc_nm_recv_cb_t cb;
-       void *cbarg;
 
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(sock->tid == isc_nm_tid());
        REQUIRE(buf != NULL);
 
-       cb = sock->recv_cb;
-       cbarg = sock->recv_cbarg;
-
        if (nread >= 0) {
                isc_region_t region = { .base = (unsigned char *)buf->base,
                                        .length = nread };
+               isc_nm_recv_cb_t cb = sock->recv_cb;
+               void *cbarg = sock->recv_cbarg;
 
                if (cb != NULL) {
                        cb(sock->statichandle, ISC_R_SUCCESS, &region, cbarg);
@@ -796,30 +816,19 @@ read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
                        uv_timer_start(&sock->timer, readtimeout_cb,
                                       sock->read_timeout, 0);
                }
+       } else {
+               /*
+                * This might happen if the inner socket is closing.  It means
+                * that it's detached, so the socket will be closed.
+                */
+               if (nread != UV_EOF) {
+                       isc__nm_incstats(sock->mgr,
+                                        sock->statsindex[STATID_RECVFAIL]);
+               }
 
-               isc__nm_free_uvbuf(sock, buf);
-               return;
+               failed_read_cb(sock, ISC_R_EOF);
        }
-
        isc__nm_free_uvbuf(sock, buf);
-
-       /*
-        * This might happen if the inner socket is closing.  It means that
-        * it's detached, so the socket will be closed.
-        */
-       if (nread != UV_EOF) {
-               isc__nm_incstats(sock->mgr, sock->statsindex[STATID_RECVFAIL]);
-       }
-       if (cb != NULL) {
-               isc__nmsocket_clearcb(sock);
-               cb(sock->statichandle, ISC_R_EOF, NULL, cbarg);
-       }
-
-       /*
-        * We don't need to clean up now; the socket will be closed and
-        * resources and quota reclaimed when handle is freed in
-        * isc__nm_tcp_close().
-        */
 }
 
 static void
@@ -985,7 +994,7 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
        return (ISC_R_SUCCESS);
 }
 
-isc_result_t
+void
 isc__nm_tcp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
                 void *cbarg) {
        isc_nmsocket_t *sock = handle->sock;
@@ -994,10 +1003,18 @@ isc__nm_tcp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
 
        REQUIRE(sock->type == isc_nm_tcpsocket);
 
+       if (!isc__nmsocket_active(sock)) {
+               isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]);
+               cb(handle, ISC_R_CANCELED, cbarg);
+               return;
+       }
+
        uvreq = isc__nm_uvreq_get(sock->mgr, sock);
        uvreq->uvbuf.base = (char *)region->base;
        uvreq->uvbuf.len = region->length;
+
        isc_nmhandle_attach(handle, &uvreq->handle);
+
        uvreq->cb.send = cb;
        uvreq->cbarg = cbarg;
 
@@ -1024,7 +1041,7 @@ isc__nm_tcp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
                isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
                                       (isc__netievent_t *)ievent);
        }
-       return (ISC_R_SUCCESS);
+       return;
 }
 
 static void
@@ -1082,8 +1099,6 @@ tcp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
        r = uv_write(&req->uv_req.write, &sock->uv_handle.stream, &req->uvbuf,
                     1, tcp_send_cb);
        if (r < 0) {
-               req->cb.send(NULL, isc__nm_uverr2result(r), req->cbarg);
-               isc__nm_uvreq_put(&req, sock);
                return (isc__nm_uverr2result(r));
        }
 
@@ -1121,6 +1136,7 @@ tcp_close_direct(isc_nmsocket_t *sock) {
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(sock->tid == isc_nm_tid());
        REQUIRE(sock->type == isc_nm_tcpsocket);
+
        if (sock->quota != NULL) {
                isc_quota_detach(&sock->quota);
        }
@@ -1128,8 +1144,11 @@ tcp_close_direct(isc_nmsocket_t *sock) {
        uv_read_stop((uv_stream_t *)&sock->uv_handle.handle);
 
        if (sock->timer_initialized) {
-               sock->timer_initialized = false;
                uv_timer_stop(&sock->timer);
+       }
+
+       if (sock->timer_initialized) {
+               sock->timer_initialized = false;
                uv_close((uv_handle_t *)&sock->timer, timer_close_cb);
        } else {
                uv_close(&sock->uv_handle.handle, tcp_close_cb);
@@ -1170,17 +1189,14 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock) {
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(sock->tid == isc_nm_tid());
 
-       if (sock->type == isc_nm_tcpsocket && sock->statichandle != NULL) {
-               isc_nm_recv_cb_t cb;
-               void *cbarg;
+       if (!isc__nmsocket_active(sock)) {
+               return;
+       }
 
-               cb = sock->recv_cb;
-               cbarg = sock->recv_cbarg;
-               isc__nmsocket_clearcb(sock);
+       atomic_store(&sock->active, false);
 
-               if (cb != NULL) {
-                       cb(sock->statichandle, ISC_R_CANCELED, NULL, cbarg);
-               }
+       if (sock->type == isc_nm_tcpsocket && sock->statichandle != NULL) {
+               failed_read_cb(sock, ISC_R_CANCELED);
        }
 }
 
@@ -1197,13 +1213,6 @@ isc__nm_tcp_cancelread(isc_nmhandle_t *handle) {
        REQUIRE(sock->tid == isc_nm_tid());
 
        if (atomic_load(&sock->client)) {
-               isc_nm_recv_cb_t cb;
-               void *cbarg;
-
-               cb = sock->recv_cb;
-               cbarg = sock->recv_cbarg;
-               isc__nmsocket_clearcb(sock);
-
-               cb(handle, ISC_R_EOF, NULL, cbarg);
+               failed_read_cb(sock, ISC_R_EOF);
        }
 }
index 283f77665ed553a9735e0e4cd214bef987d39445..dd1513f4f521c544509977ef02095b8737a37603 100644 (file)
@@ -159,10 +159,7 @@ dnslisten_acceptcb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
         * the connection is closed or there is no more data to be read.
         */
        isc_nmhandle_attach(handle, &readhandle);
-       result = isc_nm_read(readhandle, dnslisten_readcb, dnssock);
-       if (result != ISC_R_SUCCESS) {
-               isc_nmhandle_detach(&readhandle);
-       }
+       isc_nm_read(readhandle, dnslisten_readcb, dnssock);
        isc__nmsocket_detach(&dnssock);
 
        return (ISC_R_SUCCESS);
@@ -492,10 +489,7 @@ resume_processing(void *arg) {
                        }
                        isc_nmhandle_detach(&handle);
                } else if (sock->outerhandle != NULL) {
-                       result = isc_nm_resumeread(sock->outerhandle);
-                       if (result != ISC_R_SUCCESS) {
-                               isc_nmhandle_detach(&sock->outerhandle);
-                       }
+                       isc_nm_resumeread(sock->outerhandle);
                }
 
                return;
@@ -542,7 +536,6 @@ tcpdnssend_cb(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
 
 void
 isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) {
-       isc_result_t result;
        isc__netievent_tcpdnssend_t *ievent =
                (isc__netievent_tcpdnssend_t *)ev0;
        isc__nm_uvreq_t *req = ievent->req;
@@ -551,7 +544,6 @@ isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) {
        REQUIRE(worker->id == sock->tid);
        REQUIRE(sock->tid == isc_nm_tid());
 
-       result = ISC_R_NOTCONNECTED;
        if (atomic_load(&sock->active) && sock->outerhandle != NULL) {
                isc_nmhandle_t *sendhandle = NULL;
                isc_region_t r;
@@ -559,23 +551,19 @@ isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) {
                r.base = (unsigned char *)req->uvbuf.base;
                r.length = req->uvbuf.len;
                isc_nmhandle_attach(sock->outerhandle, &sendhandle);
-               result = isc_nm_send(sendhandle, &r, tcpdnssend_cb, req);
-               if (result != ISC_R_SUCCESS) {
-                       isc_nmhandle_detach(&sendhandle);
-               }
-       }
-
-       if (result != ISC_R_SUCCESS) {
-               req->cb.send(req->handle, result, req->cbarg);
-               isc_mem_put(sock->mgr->mctx, req->uvbuf.base, req->uvbuf.len);
-               isc__nm_uvreq_put(&req, sock);
+               isc_nm_send(sendhandle, &r, tcpdnssend_cb, req);
+       } else {
+               req->cb.send(req->handle, ISC_R_CANCELED, req->cbarg);
+               isc_mem_put(req->sock->mgr->mctx, req->uvbuf.base,
+                           req->uvbuf.len);
+               isc__nm_uvreq_put(&req, req->handle->sock);
        }
 }
 
 /*
  * isc__nm_tcp_send sends buf to a peer on a socket.
  */
-isc_result_t
+void
 isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
                    isc_nm_cb_t cb, void *cbarg) {
        isc__nm_uvreq_t *uvreq = NULL;
@@ -587,6 +575,11 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
        REQUIRE(VALID_NMSOCK(sock));
        REQUIRE(sock->type == isc_nm_tcpdnssocket);
 
+       if (!isc__nmsocket_active(sock)) {
+               cb(handle, ISC_R_CANCELED, cbarg);
+               return;
+       }
+
        uvreq = isc__nm_uvreq_get(sock->mgr, sock);
        isc_nmhandle_attach(handle, &uvreq->handle);
        uvreq->cb.send = cb;
@@ -598,7 +591,6 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
        memmove(uvreq->uvbuf.base + 2, region->base, region->length);
 
        if (sock->tid == isc_nm_tid()) {
-               isc_result_t result;
                isc_nmhandle_t *sendhandle = NULL;
                isc_region_t r;
 
@@ -606,12 +598,7 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
                r.length = uvreq->uvbuf.len;
 
                isc_nmhandle_attach(sock->outerhandle, &sendhandle);
-               result = isc_nm_send(sock->outerhandle, &r, tcpdnssend_cb,
-                                    uvreq);
-               if (result != ISC_R_SUCCESS) {
-                       isc_nmhandle_detach(&sendhandle);
-               }
-               return (result);
+               isc_nm_send(sock->outerhandle, &r, tcpdnssend_cb, uvreq);
        } else {
                isc__netievent_tcpdnssend_t *ievent = NULL;
 
@@ -621,11 +608,7 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
 
                isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid],
                                       (isc__netievent_t *)ievent);
-
-               return (ISC_R_SUCCESS);
        }
-
-       return (ISC_R_UNEXPECTED);
 }
 
 static void
index 373292c87a6cc17f4c1e98c2123e139ccd76dfca..dbc413649488c1ccb2b3ec20ccd7fef0aa0df1aa 100644 (file)
@@ -405,7 +405,7 @@ udp_recv_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf,
  * a proper sibling/child socket so that we won't have to jump to another
  * thread.
  */
-isc_result_t
+void
 isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
                 void *cbarg) {
        isc_nmsocket_t *sock = handle->sock;
@@ -416,6 +416,12 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
        uint32_t maxudp = atomic_load(&sock->mgr->maxudp);
        int ntid;
 
+       if (!isc__nmsocket_active(sock)) {
+               isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]);
+               cb(handle, ISC_R_CANCELED, cbarg);
+               return;
+       }
+
        /*
         * We're simulating a firewall blocking UDP packets bigger than
         * 'maxudp' bytes, for testing purposes.
@@ -426,7 +432,7 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
         */
        if (maxudp != 0 && region->length > maxudp) {
                isc_nmhandle_detach(&handle);
-               return (ISC_R_SUCCESS);
+               return;
        }
 
        if (sock->type == isc_nm_udpsocket && !atomic_load(&sock->client)) {
@@ -439,10 +445,6 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
                ISC_UNREACHABLE();
        }
 
-       if (!isc__nmsocket_active(sock)) {
-               return (ISC_R_CANCELED);
-       }
-
        /*
         * If we're in the network thread, we can send directly.  If the
         * handle is associated with a UDP socket, we can reuse its thread
@@ -472,12 +474,14 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
 
        if (isc_nm_tid() == rsock->tid) {
                /*
-                * If we're in the same thread as the socket we can send the
-                * data directly, but we need to emulate returning the error via
-                * the callback, not directly to keep the API.
+                * If we're in the same thread as the socket we can send
+                * the data directly, but we still need to return errors
+                * via the callback for API consistency.
                 */
                isc_result_t result = udp_send_direct(rsock, uvreq, peer);
                if (result != ISC_R_SUCCESS) {
+                       isc__nm_incstats(rsock->mgr,
+                                        rsock->statsindex[STATID_SENDFAIL]);
                        uvreq->cb.send(uvreq->handle, result, uvreq->cbarg);
                        isc__nm_uvreq_put(&uvreq, sock);
                }
@@ -493,8 +497,6 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb,
                isc__nm_enqueue_ievent(&sock->mgr->workers[rsock->tid],
                                       (isc__netievent_t *)ievent);
        }
-
-       return (ISC_R_SUCCESS);
 }
 
 /*
index cc28ffd0ef1ea3a3308499b21160d373952f7f02..1ca126eb47241e066951cb873dfc54420a10fd11 100644 (file)
@@ -322,20 +322,15 @@ client_allocsendbuf(ns_client_t *client, isc_buffer_t *buffer,
        *datap = data;
 }
 
-static isc_result_t
+static void
 client_sendpkg(ns_client_t *client, isc_buffer_t *buffer) {
-       isc_result_t result;
        isc_region_t r;
 
        REQUIRE(client->sendhandle == NULL);
 
        isc_buffer_usedregion(buffer, &r);
        isc_nmhandle_attach(client->handle, &client->sendhandle);
-       result = isc_nm_send(client->handle, &r, client_senddone, client);
-       if (result != ISC_R_SUCCESS) {
-               isc_nmhandle_detach(&client->sendhandle);
-       }
-       return (result);
+       isc_nm_send(client->handle, &r, client_senddone, client);
 }
 
 void
@@ -374,11 +369,9 @@ ns_client_sendraw(ns_client_t *client, dns_message_t *message) {
        r.base[0] = (client->message->id >> 8) & 0xff;
        r.base[1] = client->message->id & 0xff;
 
-       result = client_sendpkg(client, &buffer);
-       if (result == ISC_R_SUCCESS) {
-               return;
-       }
+       client_sendpkg(client, &buffer);
 
+       return;
 done:
        if (client->tcpbuf != NULL) {
                isc_mem_put(client->mctx, client->tcpbuf,
@@ -454,7 +447,7 @@ ns_client_send(ns_client_t *client) {
                result = ns_client_addopt(client, client->message,
                                          &client->opt);
                if (result != ISC_R_SUCCESS) {
-                       goto done;
+                       goto cleanup;
                }
        }
 
@@ -462,7 +455,7 @@ ns_client_send(ns_client_t *client) {
 
        result = dns_compress_init(&cctx, -1, client->mctx);
        if (result != ISC_R_SUCCESS) {
-               goto done;
+               goto cleanup;
        }
        if (client->peeraddr_valid && client->view != NULL) {
                isc_netaddr_t netaddr;
@@ -488,7 +481,7 @@ ns_client_send(ns_client_t *client) {
 
        result = dns_message_renderbegin(client->message, &cctx, &buffer);
        if (result != ISC_R_SUCCESS) {
-               goto done;
+               goto cleanup;
        }
 
        if (client->opt != NULL) {
@@ -496,7 +489,7 @@ ns_client_send(ns_client_t *client) {
                opt_included = true;
                client->opt = NULL;
                if (result != ISC_R_SUCCESS) {
-                       goto done;
+                       goto cleanup;
                }
        }
        result = dns_message_rendersection(client->message,
@@ -506,7 +499,7 @@ ns_client_send(ns_client_t *client) {
                goto renderend;
        }
        if (result != ISC_R_SUCCESS) {
-               goto done;
+               goto cleanup;
        }
        /*
         * Stop after the question if TC was set for rate limiting.
@@ -522,7 +515,7 @@ ns_client_send(ns_client_t *client) {
                goto renderend;
        }
        if (result != ISC_R_SUCCESS) {
-               goto done;
+               goto cleanup;
        }
        result = dns_message_rendersection(
                client->message, DNS_SECTION_AUTHORITY,
@@ -532,18 +525,18 @@ ns_client_send(ns_client_t *client) {
                goto renderend;
        }
        if (result != ISC_R_SUCCESS) {
-               goto done;
+               goto cleanup;
        }
        result = dns_message_rendersection(client->message,
                                           DNS_SECTION_ADDITIONAL,
                                           preferred_glue | render_opts);
        if (result != ISC_R_SUCCESS && result != ISC_R_NOSPACE) {
-               goto done;
+               goto cleanup;
        }
 renderend:
        result = dns_message_renderend(client->message);
        if (result != ISC_R_SUCCESS) {
-               goto done;
+               goto cleanup;
        }
 
 #ifdef HAVE_DNSTAP
@@ -551,13 +544,14 @@ renderend:
        if (((client->message->flags & DNS_MESSAGEFLAG_AA) != 0) &&
            (client->query.authzone != NULL))
        {
+               isc_result_t eresult;
                isc_buffer_t b;
                dns_name_t *zo = dns_zone_getorigin(client->query.authzone);
 
                isc_buffer_init(&b, zone, sizeof(zone));
                dns_compress_setmethods(&cctx, DNS_COMPRESS_NONE);
-               result = dns_name_towire(zo, &cctx, &b);
-               if (result == ISC_R_SUCCESS) {
+               eresult = dns_name_towire(zo, &cctx, &b);
+               if (eresult == ISC_R_SUCCESS) {
                        isc_buffer_usedregion(&b, &zr);
                }
        }
@@ -573,7 +567,6 @@ renderend:
 
        if (cleanup_cctx) {
                dns_compress_invalidate(&cctx);
-               cleanup_cctx = false;
        }
 
        if (client->sendcb != NULL) {
@@ -590,7 +583,7 @@ renderend:
 
                respsize = isc_buffer_usedlength(&buffer);
 
-               result = client_sendpkg(client, &buffer);
+               client_sendpkg(client, &buffer);
 
                switch (isc_sockaddr_pf(&client->peeraddr)) {
                case AF_INET:
@@ -620,7 +613,7 @@ renderend:
 
                respsize = isc_buffer_usedlength(&buffer);
 
-               result = client_sendpkg(client, &buffer);
+               client_sendpkg(client, &buffer);
 
                switch (isc_sockaddr_pf(&client->peeraddr)) {
                case AF_INET:
@@ -659,11 +652,9 @@ renderend:
                                   ns_statscounter_truncatedresp);
        }
 
-       if (result == ISC_R_SUCCESS) {
-               return;
-       }
+       return;
 
-done:
+cleanup:
        if (client->tcpbuf != NULL) {
                isc_mem_put(client->mctx, client->tcpbuf,
                            NS_CLIENT_TCP_BUFFER_SIZE);
index 5d97aff94b3fd7660e476b5c4cb25de8938a83fe..3227c5eaa79615f61c0c5d94d60fb029f1296ecd 100644 (file)
@@ -1535,8 +1535,8 @@ sendstream(xfrout_ctx_t *xfr) {
 
                isc_nmhandle_attach(xfr->client->handle,
                                    &xfr->client->sendhandle);
-               CHECK(isc_nm_send(xfr->client->sendhandle, &used,
-                                 xfrout_senddone, xfr));
+               isc_nm_send(xfr->client->sendhandle, &used, xfrout_senddone,
+                           xfr);
                xfr->sends++;
                xfr->cbytes = used.length;
        } else {