diff --git a/CHANGES b/CHANGES index e280532e3d..a0c7cce12b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5520. [bug] Fixed a number of shutdown races, reference counting + errors, and spurious log messages that could occur + in the network manager. [GL #2221] + 5519. [cleanup] Unused source code was removed: lib/dns/dbtable.c, lib/dns/portlist.c, lib/isc/bufferlist.c, and code related to those files. [GL #2060] diff --git a/bin/named/control.c b/bin/named/control.c index 3320a8fe9d..55a469925d 100644 --- a/bin/named/control.c +++ b/bin/named/control.c @@ -183,8 +183,7 @@ named_control_docommand(isccc_sexpr_t *message, bool readonly, /* Do not flush master files */ named_server_flushonshutdown(named_g_server, false); named_os_shutdownmsg(cmdline, *text); - isc_app_shutdown(); - result = ISC_R_SUCCESS; + result = ISC_R_SHUTTINGDOWN; } else if (command_compare(command, NAMED_COMMAND_STOP)) { /* * "stop" is the same as "halt" except it does @@ -201,8 +200,7 @@ named_control_docommand(isccc_sexpr_t *message, bool readonly, #endif /* ifdef HAVE_LIBSCF */ named_server_flushonshutdown(named_g_server, true); named_os_shutdownmsg(cmdline, *text); - isc_app_shutdown(); - result = ISC_R_SUCCESS; + result = ISC_R_SHUTTINGDOWN; } else if (command_compare(command, NAMED_COMMAND_ADDZONE) || command_compare(command, NAMED_COMMAND_MODZONE)) { diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index deb1e7af67..871cb459b7 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -225,6 +226,11 @@ control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { conn->sending = false; + if (conn->result == ISC_R_SHUTTINGDOWN) { + isc_app_shutdown(); + goto cleanup_sendhandle; + } + if (atomic_load_acquire(&listener->controls->shuttingdown) || result == ISC_R_CANCELED) { @@ -245,14 +251,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: @@ -291,12 +290,12 @@ conn_cleanup(controlconnection_t *conn) { } static void -control_respond(isc_nmhandle_t *handle, isc_result_t result, void *arg) { - controlconnection_t *conn = (controlconnection_t *)arg; +control_respond(isc_nmhandle_t *handle, controlconnection_t *conn) { controllistener_t *listener = conn->listener; isccc_sexpr_t *data = NULL; isc_buffer_t b; isc_region_t r; + isc_result_t result; result = isccc_cc_createresponse(conn->request, conn->now, conn->now + 60, &conn->response); @@ -304,17 +303,22 @@ control_respond(isc_nmhandle_t *handle, isc_result_t result, void *arg) { goto cleanup; } + if (conn->result == ISC_R_SHUTTINGDOWN) { + result = ISC_R_SUCCESS; + } else { + result = conn->result; + } + data = isccc_alist_lookup(conn->response, "_data"); if (data != NULL) { - if (isccc_cc_defineuint32(data, "result", conn->result) == NULL) - { + if (isccc_cc_defineuint32(data, "result", result) == NULL) { goto cleanup; } } - if (conn->result != ISC_R_SUCCESS) { + if (result != ISC_R_SUCCESS) { if (data != NULL) { - const char *estr = isc_result_totext(conn->result); + const char *estr = isc_result_totext(result); if (isccc_cc_definestring(data, "err", estr) == NULL) { goto cleanup; } @@ -363,11 +367,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; @@ -391,7 +391,7 @@ control_command(isc_task_t *task, isc_event_t *event) { conn->result = named_control_docommand(conn->request, listener->readonly, &conn->text); - control_respond(conn->cmdhandle, conn->result, conn); + control_respond(conn->cmdhandle, conn); done: isc_event_free(&event); @@ -532,7 +532,7 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isc_nonce_buf(&conn->nonce, sizeof(conn->nonce)); } conn->result = ISC_R_SUCCESS; - control_respond(handle, result, conn); + control_respond(handle, conn); return; } @@ -596,10 +596,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 +626,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 +652,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 170fd1fff2..a8f7277a01 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 5ba984d406..ca304e973b 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 dc4f977e42..8843cbd54f 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 @@ -208,7 +208,7 @@ isc_nm_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg); * is data to process. */ -isc_result_t +void isc_nm_pauseread(isc_nmhandle_t *handle); /*%< * Pause reading on this handle's socket, but remember the callback. @@ -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 b9a9672783..99ce506987 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -660,6 +660,18 @@ isc__nmsocket_active(isc_nmsocket_t *sock); * or, for child sockets, 'sock->parent->active'. */ +bool +isc__nmsocket_deactivate(isc_nmsocket_t *sock); +/*%< + * @brief Deactivate active socket + * + * Atomically deactive the socket by setting @p sock->active or, for child + * sockets, @p sock->parent->active to @c false + * + * @param[in] sock - valid nmsocket + * @return @c false if the socket was already inactive, @c true otherwise + */ + void isc__nmsocket_clearcb(isc_nmsocket_t *sock); /*%< @@ -679,7 +691,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 +712,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. @@ -718,13 +730,13 @@ isc__nm_tcp_close(isc_nmsocket_t *sock); /*%< * Close a TCP socket. */ -isc_result_t +void 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 +786,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 8358ee4e61..cb6b18911c 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); /* @@ -728,6 +731,19 @@ isc__nmsocket_active(isc_nmsocket_t *sock) { return (atomic_load(&sock->active)); } +bool +isc__nmsocket_deactivate(isc_nmsocket_t *sock) { + REQUIRE(VALID_NMSOCK(sock)); + + if (sock->parent != NULL) { + return (atomic_compare_exchange_strong(&sock->parent->active, + &(bool){ true }, false)); + } + + return (atomic_compare_exchange_strong(&sock->active, &(bool){ true }, + false)); +} + void isc__nmsocket_attach(isc_nmsocket_t *sock, isc_nmsocket_t **target) { REQUIRE(VALID_NMSOCK(sock)); @@ -1377,7 +1393,7 @@ isc__nm_uvreq_get(isc_nm_t *mgr, isc_nmsocket_t *sock) { REQUIRE(VALID_NM(mgr)); REQUIRE(VALID_NMSOCK(sock)); - if (sock != NULL && atomic_load(&sock->active)) { + if (sock != NULL && isc__nmsocket_active(sock)) { /* Try to reuse one */ req = isc_astack_pop(sock->inactivereqs); } @@ -1416,7 +1432,7 @@ isc__nm_uvreq_put(isc__nm_uvreq_t **req0, isc_nmsocket_t *sock) { handle = req->handle; req->handle = NULL; - if (!atomic_load(&sock->active) || + if (!isc__nmsocket_active(sock) || !isc_astack_trypush(sock->inactivereqs, req)) { isc_mempool_put(sock->mgr->reqpool, req); } @@ -1428,7 +1444,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 +1452,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(); @@ -1474,7 +1494,7 @@ isc_nm_cancelread(isc_nmhandle_t *handle) { } } -isc_result_t +void isc_nm_pauseread(isc_nmhandle_t *handle) { REQUIRE(VALID_NMHANDLE(handle)); @@ -1482,14 +1502,15 @@ isc_nm_pauseread(isc_nmhandle_t *handle) { switch (sock->type) { case isc_nm_tcpsocket: - return (isc__nm_tcp_pauseread(sock)); + isc__nm_tcp_pauseread(sock); + break; default: INSIST(0); ISC_UNREACHABLE(); } } -isc_result_t +void isc_nm_resumeread(isc_nmhandle_t *handle) { REQUIRE(VALID_NMHANDLE(handle)); @@ -1497,7 +1518,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 +1860,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 +1873,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 483bd6d12f..43d3f9fc9f 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -134,12 +135,15 @@ isc__nm_async_tcpconnect(isc__networker_t *worker, isc__netievent_t *ev0) { if (r != 0) { /* We need to issue callbacks ourselves */ tcp_connect_cb(&req->uv_req.connect, r); - goto done; + LOCK(&sock->lock); + SIGNAL(&sock->cond); + UNLOCK(&sock->lock); + isc__nmsocket_detach(&sock); + return; } atomic_store(&sock->connected, true); -done: LOCK(&sock->lock); SIGNAL(&sock->cond); UNLOCK(&sock->lock); @@ -160,6 +164,7 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) { if (status != 0) { req->cb.connect(NULL, isc__nm_uverr2result(status), req->cbarg); isc__nm_uvreq_put(&req, sock); + isc__nmsocket_detach(&sock); return; } @@ -242,7 +247,6 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_nmiface_t *local, isc_nmiface_t *peer, if (nsock->result != ISC_R_SUCCESS) { result = nsock->result; - isc__nmsocket_detach(&nsock); } isc__nmsocket_detach(&tmp); @@ -513,9 +517,17 @@ error: if (sock->quota != NULL) { isc_quota_detach(&sock->quota); } - isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_NETMGR, - ISC_LOG_ERROR, "Accepting TCP connection failed: %s", - isc_result_totext(result)); + + switch (result) { + case ISC_R_NOTCONNECTED: + /* IGNORE: The client disconnected before we could accept */ + break; + default: + isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL, + ISC_LOGMODULE_NETMGR, ISC_LOG_ERROR, + "Accepting TCP connection failed: %s", + isc_result_totext(result)); + } /* * Detach the socket properly to make sure uv_close() is called. @@ -579,11 +591,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()); @@ -600,29 +634,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; @@ -640,7 +667,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; } /*%< @@ -674,6 +701,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); @@ -683,24 +720,19 @@ 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 +void isc__nm_tcp_pauseread(isc_nmsocket_t *sock) { isc__netievent_pauseread_t *ievent = NULL; REQUIRE(VALID_NMSOCK(sock)); - if (atomic_load(&sock->readpaused)) { - return (ISC_R_SUCCESS); + if (!atomic_compare_exchange_strong(&sock->readpaused, &(bool){ false }, + true)) { + return; } - atomic_store(&sock->readpaused, true); ievent = isc__nm_get_ievent(sock->mgr, netievent_tcppauseread); ievent->sock = sock; @@ -713,7 +745,7 @@ isc__nm_tcp_pauseread(isc_nmsocket_t *sock) { (isc__netievent_t *)ievent); } - return (ISC_R_SUCCESS); + return; } void @@ -730,7 +762,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; @@ -738,14 +770,18 @@ 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 (!atomic_load(&sock->readpaused)) { - return (ISC_R_SUCCESS); + if (!isc__nmsocket_active(sock)) { + failed_read_cb(sock, ISC_R_CANCELED); + return; } - atomic_store(&sock->readpaused, false); + if (!atomic_compare_exchange_strong(&sock->readpaused, &(bool){ true }, + false)) { + return; + } ievent = isc__nm_get_ievent(sock->mgr, netievent_tcpstartread); ievent->sock = sock; @@ -758,26 +794,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); @@ -792,30 +823,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 @@ -882,9 +902,7 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { REQUIRE(VALID_NMSOCK(ssock)); - if (!atomic_load_relaxed(&ssock->active) || - atomic_load_relaxed(&ssock->mgr->closing)) - { + if (!isc__nmsocket_active(ssock) || atomic_load(&ssock->mgr->closing)) { /* We're closing, bail */ if (quota != NULL) { isc_quota_detach("a); @@ -933,7 +951,9 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { if (r != 0) { result = isc__nm_uverr2result(r); uv_close((uv_handle_t *)uvstream, free_uvtcpt); - isc_quota_detach("a); + if (quota != NULL) { + isc_quota_detach("a); + } return (result); } @@ -942,6 +962,17 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { event = isc__nm_get_ievent(ssock->mgr, netievent_tcpchildaccept); /* Duplicate the server socket */ + r = isc_uv_export((uv_stream_t *)uvstream, &event->streaminfo); + if (r != 0) { + result = isc_errno_toresult(errno); + uv_close((uv_handle_t *)uvstream, free_uvtcpt); + if (quota != NULL) { + isc_quota_detach("a); + } + isc__nm_put_ievent(ssock->mgr, event); + return (result); + } + isc_nmsocket_t *csock = isc_mem_get(ssock->mgr->mctx, sizeof(isc_nmsocket_t)); isc__nmsocket_init(csock, ssock->mgr, isc_nm_tcpsocket, ssock->iface); @@ -954,9 +985,6 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { event->sock = csock; event->quota = quota; - r = isc_uv_export((uv_stream_t *)uvstream, &event->streaminfo); - RUNTIME_CHECK(r == 0); - uv_close((uv_handle_t *)uvstream, free_uvtcpt); if (w == isc_nm_tid()) { @@ -971,7 +999,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; @@ -980,10 +1008,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; @@ -992,7 +1028,13 @@ isc__nm_tcp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, * If we're in the same thread as the socket we can send the * data directly */ - return (tcp_send_direct(sock, uvreq)); + isc_result_t result = tcp_send_direct(sock, uvreq); + if (result != ISC_R_SUCCESS) { + isc__nm_incstats(sock->mgr, + sock->statsindex[STATID_SENDFAIL]); + uvreq->cb.send(uvreq->handle, result, uvreq->cbarg); + isc__nm_uvreq_put(&uvreq, sock); + } } else { /* * We need to create an event and pass it using async channel @@ -1000,32 +1042,28 @@ isc__nm_tcp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, ievent = isc__nm_get_ievent(sock->mgr, netievent_tcpsend); ievent->sock = sock; ievent->req = uvreq; + isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid], (isc__netievent_t *)ievent); - return (ISC_R_SUCCESS); } - - return (ISC_R_UNEXPECTED); + return; } static void tcp_send_cb(uv_write_t *req, int status) { isc_result_t result = ISC_R_SUCCESS; isc__nm_uvreq_t *uvreq = (isc__nm_uvreq_t *)req->data; - isc_nmsocket_t *sock = NULL; + isc_nmsocket_t *sock = uvreq->sock; REQUIRE(VALID_UVREQ(uvreq)); REQUIRE(VALID_NMHANDLE(uvreq->handle)); if (status < 0) { result = isc__nm_uverr2result(status); - isc__nm_incstats(uvreq->sock->mgr, - uvreq->sock->statsindex[STATID_SENDFAIL]); + isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]); } uvreq->cb.send(uvreq->handle, result, uvreq->cbarg); - - sock = uvreq->handle->sock; isc__nm_uvreq_put(&uvreq, sock); } @@ -1036,18 +1074,17 @@ void isc__nm_async_tcpsend(isc__networker_t *worker, isc__netievent_t *ev0) { isc_result_t result; isc__netievent_tcpsend_t *ievent = (isc__netievent_tcpsend_t *)ev0; + isc_nmsocket_t *sock = ievent->sock; + isc__nm_uvreq_t *uvreq = ievent->req; + REQUIRE(sock->type == isc_nm_tcpsocket); REQUIRE(worker->id == ievent->sock->tid); - if (!atomic_load(&ievent->sock->active)) { - return; - } - - result = tcp_send_direct(ievent->sock, ievent->req); + result = tcp_send_direct(sock, uvreq); if (result != ISC_R_SUCCESS) { - ievent->req->cb.send(ievent->req->handle, result, - ievent->req->cbarg); - isc__nm_uvreq_put(&ievent->req, ievent->req->handle->sock); + isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]); + uvreq->cb.send(uvreq->handle, result, uvreq->cbarg); + isc__nm_uvreq_put(&uvreq, sock); } } @@ -1055,14 +1092,18 @@ static isc_result_t tcp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { int r; + REQUIRE(VALID_NMSOCK(sock)); + REQUIRE(VALID_UVREQ(req)); REQUIRE(sock->tid == isc_nm_tid()); REQUIRE(sock->type == isc_nm_tcpsocket); + + if (!isc__nmsocket_active(sock)) { + return (ISC_R_CANCELED); + } + r = uv_write(&req->uv_req.write, &sock->uv_handle.stream, &req->uvbuf, 1, tcp_send_cb); if (r < 0) { - isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]); - req->cb.send(NULL, isc__nm_uverr2result(r), req->cbarg); - isc__nm_uvreq_put(&req, sock); return (isc__nm_uverr2result(r)); } @@ -1078,6 +1119,11 @@ tcp_close_cb(uv_handle_t *uvhandle) { isc__nm_incstats(sock->mgr, sock->statsindex[STATID_CLOSE]); atomic_store(&sock->closed, true); atomic_store(&sock->connected, false); + + if (sock->server != NULL) { + isc__nmsocket_detach(&sock->server); + } + isc__nmsocket_prep_destroy(sock); } @@ -1087,9 +1133,6 @@ timer_close_cb(uv_handle_t *uvhandle) { REQUIRE(VALID_NMSOCK(sock)); - if (sock->server != NULL) { - isc__nmsocket_detach(&sock->server); - } uv_close(&sock->uv_handle.handle, tcp_close_cb); } @@ -1098,17 +1141,21 @@ 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); } + + uv_read_stop((uv_stream_t *)&sock->uv_handle.handle); + + if (sock->timer_initialized) { + uv_timer_stop(&sock->timer); + } + if (sock->timer_initialized) { sock->timer_initialized = false; - uv_timer_stop(&sock->timer); uv_close((uv_handle_t *)&sock->timer, timer_close_cb); } else { - if (sock->server != NULL) { - isc__nmsocket_detach(&sock->server); - } uv_close(&sock->uv_handle.handle, tcp_close_cb); } } @@ -1147,17 +1194,16 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_nm_tid()); + /* + * If the socket is active, mark it inactive and + * continue. If it isn't active, stop now. + */ + if (!isc__nmsocket_deactivate(sock)) { + return; + } + if (sock->type == isc_nm_tcpsocket && sock->statichandle != NULL) { - isc_nm_recv_cb_t cb; - void *cbarg; - - cb = sock->recv_cb; - cbarg = sock->recv_cbarg; - isc__nmsocket_clearcb(sock); - - if (cb != NULL) { - cb(sock->statichandle, ISC_R_CANCELED, NULL, cbarg); - } + failed_read_cb(sock, ISC_R_CANCELED); } } @@ -1174,13 +1220,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 283f77665e..6a20ea4c7c 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,31 +544,26 @@ 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) { + if (isc__nmsocket_active(sock) && sock->outerhandle != NULL) { isc_nmhandle_t *sendhandle = NULL; isc_region_t r; 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 f87ab95a5f..d97a0b612a 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -262,16 +262,12 @@ isc__nm_udp_stoplistening(isc_nmsocket_t *sock) { REQUIRE(sock->type == isc_nm_udplistener); /* - * Socket is already closing; there's nothing to do. + * If the socket is active, mark it inactive and + * continue. If it isn't active, stop now. */ - if (!isc__nmsocket_active(sock)) { + if (!isc__nmsocket_deactivate(sock)) { return; } - /* - * Mark it inactive now so that all sends will be ignored - * and we won't try to stop listening again. - */ - atomic_store(&sock->active, false); /* * If the manager is interlocked, re-enqueue this as an asynchronous @@ -407,7 +403,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 +414,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 +430,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 +443,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,10 +472,17 @@ 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 + * 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. */ - return (udp_send_direct(rsock, uvreq, peer)); + 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); + } } else { /* * We need to create an event and pass it using async channel @@ -489,7 +494,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); } } @@ -498,16 +502,19 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, */ void isc__nm_async_udpsend(isc__networker_t *worker, isc__netievent_t *ev0) { + isc_result_t result; isc__netievent_udpsend_t *ievent = (isc__netievent_udpsend_t *)ev0; + isc_nmsocket_t *sock = ievent->sock; + isc__nm_uvreq_t *uvreq = ievent->req; - REQUIRE(worker->id == ievent->sock->tid); + REQUIRE(sock->type == isc_nm_udpsocket); + REQUIRE(worker->id == sock->tid); - if (isc__nmsocket_active(ievent->sock)) { - udp_send_direct(ievent->sock, ievent->req, &ievent->peer); - } else { - ievent->req->cb.send(ievent->req->handle, ISC_R_CANCELED, - ievent->req->cbarg); - isc__nm_uvreq_put(&ievent->req, ievent->req->sock); + result = udp_send_direct(sock, uvreq, &ievent->peer); + if (result != ISC_R_SUCCESS) { + isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]); + uvreq->cb.send(uvreq->handle, result, uvreq->cbarg); + isc__nm_uvreq_put(&uvreq, sock); } } @@ -515,14 +522,14 @@ static void udp_send_cb(uv_udp_send_t *req, int status) { isc_result_t result = ISC_R_SUCCESS; isc__nm_uvreq_t *uvreq = (isc__nm_uvreq_t *)req->data; + isc_nmsocket_t *sock = uvreq->sock; REQUIRE(VALID_UVREQ(uvreq)); REQUIRE(VALID_NMHANDLE(uvreq->handle)); if (status < 0) { result = isc__nm_uverr2result(status); - isc__nm_incstats(uvreq->sock->mgr, - uvreq->sock->statsindex[STATID_SENDFAIL]); + isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]); } uvreq->cb.send(uvreq->handle, result, uvreq->cbarg); @@ -537,8 +544,10 @@ static isc_result_t udp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req, isc_sockaddr_t *peer) { const struct sockaddr *sa = NULL; - int rv; + int r; + REQUIRE(VALID_NMSOCK(sock)); + REQUIRE(VALID_UVREQ(req)); REQUIRE(sock->tid == isc_nm_tid()); REQUIRE(sock->type == isc_nm_udpsocket); @@ -547,12 +556,10 @@ udp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req, } sa = atomic_load(&sock->connected) ? NULL : &peer->type.sa; - rv = uv_udp_send(&req->uv_req.udp_send, &sock->uv_handle.udp, - &req->uvbuf, 1, sa, udp_send_cb); - if (rv < 0) { - isc__nm_incstats(req->sock->mgr, - req->sock->statsindex[STATID_SENDFAIL]); - return (isc__nm_uverr2result(rv)); + r = uv_udp_send(&req->uv_req.udp_send, &sock->uv_handle.udp, + &req->uvbuf, 1, sa, udp_send_cb); + if (r < 0) { + return (isc__nm_uverr2result(r)); } return (ISC_R_SUCCESS); diff --git a/lib/isccc/ccmsg.c b/lib/isccc/ccmsg.c index 3cb0732f94..649b3f332b 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 a793a96bfc..c5ea35d088 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 a2eed9250d..cf9f5dc137 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 9eb283ff00..0b2d1c4f8c 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 {