From: Ondřej Surý Date: Wed, 21 Oct 2020 10:52:09 +0000 (+0200) Subject: Fix the isc_nm_closedown() to actually close the pending connections X-Git-Tag: v9.17.7~46^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f7c82e406e83f98d3a1fecedc78725b441552092;p=thirdparty%2Fbind9.git Fix the isc_nm_closedown() to actually close the pending connections 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). --- diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index deb1e7af67e..4bfa6af4c18 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -245,14 +245,7 @@ control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isc_nmhandle_detach(&conn->sendhandle); - result = isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, - conn); - if (result != ISC_R_SUCCESS) { - conn->reading = false; - isc_nmhandle_detach(&conn->readhandle); - return; - } - + isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, conn); return; cleanup_sendhandle: @@ -363,11 +356,7 @@ control_respond(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isc_nmhandle_detach(&conn->cmdhandle); - result = isc_nm_send(conn->sendhandle, &r, control_senddone, conn); - if (result != ISC_R_SUCCESS) { - isc_nmhandle_detach(&conn->sendhandle); - conn->sending = false; - } + isc_nm_send(conn->sendhandle, &r, control_senddone, conn); return; @@ -596,10 +585,9 @@ conn_put(void *arg) { maybe_free_listener(listener); } -static isc_result_t +static void newconnection(controllistener_t *listener, isc_nmhandle_t *handle) { controlconnection_t *conn = NULL; - isc_result_t result; conn = isc_nmhandle_getdata(handle); if (conn == NULL) { @@ -627,26 +615,7 @@ newconnection(controllistener_t *listener, isc_nmhandle_t *handle) { isc_nmhandle_attach(handle, &conn->readhandle); conn->reading = true; - result = isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, - conn); - if (result != ISC_R_SUCCESS) { - isc_nmhandle_detach(&conn->readhandle); - conn->reading = false; - goto cleanup; - } - - return (ISC_R_SUCCESS); - -cleanup: - /* - * conn_reset() handles the cleanup. - */ -#ifdef ENABLE_AFL - if (named_g_fuzz_type == isc_fuzz_rndc) { - named_fuzz_notify(); - } -#endif /* ifdef ENABLE_AFL */ - return (result); + isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, conn); } static isc_result_t @@ -672,17 +641,7 @@ control_newconn(isc_nmhandle_t *handle, isc_result_t result, void *arg) { return (ISC_R_FAILURE); } - result = newconnection(listener, handle); - if (result != ISC_R_SUCCESS) { - char socktext[ISC_SOCKADDR_FORMATSIZE]; - isc_sockaddr_format(&peeraddr, socktext, sizeof(socktext)); - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_CONTROL, ISC_LOG_WARNING, - "dropped command channel from %s: %s", socktext, - isc_result_totext(result)); - return (result); - } - + newconnection(listener, handle); return (ISC_R_SUCCESS); } diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c index 170fd1fff2c..a8f7277a017 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -402,13 +402,14 @@ static void rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isccc_ccmsg_t *ccmsg = (isccc_ccmsg_t *)arg; isccc_sexpr_t *response = NULL; - isccc_sexpr_t *_ctrl; + isc_nmhandle_t *sendhandle = NULL; + isccc_sexpr_t *_ctrl = NULL; isccc_region_t source; uint32_t nonce; isccc_sexpr_t *request = NULL; isccc_time_t now; isc_region_t r; - isccc_sexpr_t *data; + isccc_sexpr_t *data = NULL; isc_buffer_t b; REQUIRE(ccmsg != NULL); @@ -484,13 +485,11 @@ rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isc_nmhandle_attach(handle, &recvdone_handle); atomic_fetch_add_relaxed(&recvs, 1); - DO("schedule recv", - isccc_ccmsg_readmessage(ccmsg, rndc_recvdone, ccmsg)); + isccc_ccmsg_readmessage(ccmsg, rndc_recvdone, ccmsg); - isc_nmhandle_t *sendhandle = NULL; isc_nmhandle_attach(handle, &sendhandle); atomic_fetch_add_relaxed(&sends, 1); - DO("send message", isc_nm_send(handle, &r, rndc_senddone, sendhandle)); + isc_nm_send(handle, &r, rndc_senddone, sendhandle); REQUIRE(recvnonce_handle == handle); isc_nmhandle_detach(&recvnonce_handle); @@ -506,11 +505,12 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isccc_ccmsg_t *ccmsg = (isccc_ccmsg_t *)arg; char socktext[ISC_SOCKADDR_FORMATSIZE]; isccc_sexpr_t *request = NULL; - isccc_sexpr_t *data; + isccc_sexpr_t *data = NULL; isccc_time_t now; isc_region_t r; isc_buffer_t b; isc_nmhandle_t *connhandle = NULL; + isc_nmhandle_t *sendhandle = NULL; REQUIRE(ccmsg != NULL); @@ -560,13 +560,11 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isc_nmhandle_attach(handle, &recvnonce_handle); atomic_fetch_add_relaxed(&recvs, 1); - DO("schedule recv", - isccc_ccmsg_readmessage(ccmsg, rndc_recvnonce, ccmsg)); + isccc_ccmsg_readmessage(ccmsg, rndc_recvnonce, ccmsg); - isc_nmhandle_t *sendhandle = NULL; isc_nmhandle_attach(handle, &sendhandle); atomic_fetch_add_relaxed(&sends, 1); - DO("send message", isc_nm_send(handle, &r, rndc_senddone, sendhandle)); + isc_nm_send(handle, &r, rndc_senddone, sendhandle); isc_nmhandle_detach(&connhandle); atomic_fetch_sub_release(&connects, 1); diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index 5ba984d406e..ca304e973b1 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -623,9 +623,8 @@ httpd_put(void *arg) { #endif /* ENABLE_AFL */ } -static isc_result_t +static void new_httpd(isc_httpdmgr_t *httpdmgr, isc_nmhandle_t *handle) { - isc_result_t result; isc_httpd_t *httpd = NULL; char *headerdata = NULL; @@ -668,12 +667,7 @@ new_httpd(isc_httpdmgr_t *httpdmgr, isc_nmhandle_t *handle) { UNLOCK(&httpdmgr->lock); isc_nmhandle_attach(handle, &httpd->readhandle); - result = isc_nm_read(handle, httpd_request, httpdmgr); - if (result != ISC_R_SUCCESS) { - isc_nmhandle_detach(&httpd->readhandle); - } - - return (result); + isc_nm_read(handle, httpd_request, httpdmgr); } static isc_result_t @@ -699,7 +693,9 @@ httpd_newconn(isc_nmhandle_t *handle, isc_result_t result, void *arg) { return (ISC_R_FAILURE); } - return (new_httpd(httpdmgr, handle)); + new_httpd(httpdmgr, handle); + + return (ISC_R_SUCCESS); } static isc_result_t @@ -963,13 +959,7 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, httpd->state = SEND; isc_nmhandle_attach(handle, &httpd->sendhandle); - result = isc_nm_send(handle, &r, httpd_senddone, httpd); - if (result != ISC_R_SUCCESS) { - isc_nmhandle_detach(&httpd->sendhandle); - - httpd->state = RECV; - isc_nm_resumeread(handle); - } + isc_nm_send(handle, &r, httpd_senddone, httpd); return; cleanup_readhandle: @@ -1137,12 +1127,12 @@ httpd_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { } } + isc_nmhandle_detach(&httpd->sendhandle); + if (result != ISC_R_SUCCESS) { return; } - isc_nmhandle_detach(&httpd->sendhandle); - httpd->state = RECV; isc_nm_resumeread(handle); } diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index dc4f977e420..95822d1c783 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -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); /*%< diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index b9a96727838..dd1366970ab 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -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); /*%< diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 8358ee4e61a..4e945ba082a 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -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); diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 872c137f0a9..5129e1d4a54 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -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, ®ion, 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); } } diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index 283f77665ed..dd1513f4f52 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -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 diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index ed49b47274b..d11b13bfe66 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -407,7 +407,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; @@ -418,6 +418,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. @@ -428,7 +434,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)) { @@ -441,10 +447,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 @@ -474,12 +476,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); } @@ -495,8 +499,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); } /* diff --git a/lib/isccc/ccmsg.c b/lib/isccc/ccmsg.c index 3cb0732f945..649b3f332bd 100644 --- a/lib/isccc/ccmsg.c +++ b/lib/isccc/ccmsg.c @@ -133,10 +133,8 @@ isccc_ccmsg_setmaxsize(isccc_ccmsg_t *ccmsg, unsigned int maxsize) { ccmsg->maxsize = maxsize; } -isc_result_t +void isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg) { - isc_result_t result; - REQUIRE(VALID_CCMSG(ccmsg)); if (ccmsg->buffer != NULL) { @@ -149,16 +147,11 @@ isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg) { ccmsg->length_received = false; if (ccmsg->reading) { - result = isc_nm_resumeread(ccmsg->handle); + isc_nm_resumeread(ccmsg->handle); } else { - result = isc_nm_read(ccmsg->handle, recv_data, ccmsg); + isc_nm_read(ccmsg->handle, recv_data, ccmsg); ccmsg->reading = true; } - if (result != ISC_R_SUCCESS) { - ccmsg->reading = false; - } - - return (result); } void diff --git a/lib/isccc/include/isccc/ccmsg.h b/lib/isccc/include/isccc/ccmsg.h index a793a96bfcb..c5ea35d088e 100644 --- a/lib/isccc/include/isccc/ccmsg.h +++ b/lib/isccc/include/isccc/ccmsg.h @@ -87,7 +87,7 @@ isccc_ccmsg_setmaxsize(isccc_ccmsg_t *ccmsg, unsigned int maxsize); *\li 512 <= "maxsize" <= 4294967296 */ -isc_result_t +void isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg); /*% * Schedule an event to be delivered when a command channel message is @@ -97,11 +97,6 @@ isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg); * *\li "ccmsg" be valid. * - * Returns: - * - *\li #ISC_R_SUCCESS -- no error - *\li Anything that the isc_nm_read() call can return. - * * Notes: * *\li The event delivered is a fully generic event. It will contain no diff --git a/lib/ns/client.c b/lib/ns/client.c index a2eed9250d7..cf9f5dc1375 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -323,20 +323,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 @@ -375,11 +370,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, @@ -455,7 +448,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; } } @@ -463,7 +456,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; @@ -489,7 +482,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) { @@ -497,7 +490,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, @@ -507,7 +500,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. @@ -523,7 +516,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, @@ -533,18 +526,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 @@ -552,13 +545,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); } } @@ -574,7 +568,6 @@ renderend: if (cleanup_cctx) { dns_compress_invalidate(&cctx); - cleanup_cctx = false; } if (client->sendcb != NULL) { @@ -591,7 +584,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: @@ -621,7 +614,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: @@ -660,11 +653,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); diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c index 9eb283ff001..0b2d1c4f8c1 100644 --- a/lib/ns/xfrout.c +++ b/lib/ns/xfrout.c @@ -1567,8 +1567,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 {