From: Ondřej Surý Date: Thu, 13 Apr 2023 15:27:50 +0000 (+0200) Subject: Fix the streaming read callback shutdown logic X-Git-Tag: v9.19.13~22^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3b108145694bbb1f6459f5742569b2f0ceea9209;p=thirdparty%2Fbind9.git Fix the streaming read callback shutdown logic When shutting down TCP sockets, the read callback calling logic was flawed, it would call either one less callback or one extra. Fix the logic in the way: 1. When isc_nm_read() has been called but isc_nm_read_stop() hasn't on the handle, the read callback will be called with ISC_R_CANCELED to cancel active reading from the socket/handle. 2. When isc_nm_read() has been called and isc_nm_read_stop() has been called on the on the handle, the read callback will be called with ISC_R_SHUTTINGDOWN to signal that the dormant (not-reading) socket is being shut down. 3. The .reading and .recv_read flags are little bit tricky. The .reading flag indicates if the outer layer is reading the data (that would be uv_tcp_t for TCP and isc_nmsocket_t (TCP) for TLSStream), the .recv_read flag indicates whether somebody is interested in the data read from the socket. Usually, you would expect that the .reading should be false when .recv_read is false, but it gets even more tricky with TLSStream as the TLS protocol might need to read from the socket even when sending data. Fix the usage of the .recv_read and .reading flags in the TLSStream to their true meaning - which mostly consist of using .recv_read everywhere and then wrapping isc_nm_read() and isc_nm_read_stop() with the .reading flag. 4. The TLS failed read helper has been modified to resemble the TCP code as much as possible, clearing and re-setting the .recv_read flag in the TCP timeout code has been fixed and .recv_read is now cleared when isc_nm_read_stop() has been called on the streaming socket. 5. The use of Network Manager in the named_controlconf, isccc_ccmsg, and isc_httpd units have been greatly simplified due to the improved design. 6. More unit tests for TCP and TLS testing the shutdown conditions have been added. Co-authored-by: Ondřej Surý Co-authored-by: Artem Boldariev --- diff --git a/bin/named/controlconf.c b/bin/named/controlconf.c index 56084de881d..fd1201c41fe 100644 --- a/bin/named/controlconf.c +++ b/bin/named/controlconf.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -48,6 +49,8 @@ #include #include +#undef NAMED_CONTROLCONF_TRACE + typedef struct controlkey controlkey_t; typedef ISC_LIST(controlkey_t) controlkeylist_t; @@ -65,12 +68,8 @@ struct controlkey { }; struct controlconnection { - isc_nmhandle_t *readhandle; - isc_nmhandle_t *sendhandle; - isc_nmhandle_t *cmdhandle; + isc_refcount_t references; isccc_ccmsg_t ccmsg; - bool reading; - bool sending; controllistener_t *listener; isccc_sexpr_t *ctrl; isc_buffer_t *buffer; @@ -83,6 +82,7 @@ struct controlconnection { isc_stdtime_t now; isc_result_t result; ISC_LINK(controlconnection_t) link; + bool shuttingdown; }; struct controllistener { @@ -92,7 +92,7 @@ struct controllistener { isc_nmsocket_t *sock; dns_acl_t *acl; bool exiting; - isc_refcount_t refs; + isc_refcount_t references; controlkeylist_t keys; isc_mutex_t connections_lock; controlconnectionlist_t connections; @@ -107,7 +107,7 @@ struct controllistener { struct named_controls { named_server_t *server; controllistenerlist_t listeners; - atomic_bool shuttingdown; + bool shuttingdown; isc_mutex_t symtab_lock; isccc_symtab_t *symtab; }; @@ -116,6 +116,35 @@ static isc_result_t control_newconn(isc_nmhandle_t *handle, isc_result_t result, void *arg); static void control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg); +static void +conn_cleanup(controlconnection_t *conn); +static void +conn_free(controlconnection_t *conn); + +#if NAMED_CONTROLCONF_TRACE +#define controllistener_ref(ptr) \ + controllistener__ref(ptr, __func__, __FILE__, __LINE__) +#define controllistener_unref(ptr) \ + controllistener__unref(ptr, __func__, __FILE__, __LINE__) +#define controllistener_attach(ptr, ptrp) \ + controllistener__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define controllistener_detach(ptrp) \ + controllistener__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(controllistener); + +#define controlconnection_ref(ptr) \ + controlconnection__ref(ptr, __func__, __FILE__, __LINE__) +#define controlconnection_unref(ptr) \ + controlconnection__unref(ptr, __func__, __FILE__, __LINE__) +#define controlconnection_attach(ptr, ptrp) \ + controlconnection__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define controlconnection_detach(ptrp) \ + controlconnection__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(controlconnection); +#else +ISC_REFCOUNT_DECL(controllistener); +ISC_REFCOUNT_DECL(controlconnection); +#endif #define CLOCKSKEW 300 @@ -144,7 +173,7 @@ free_listener(controllistener_t *listener) { INSIST(listener->exiting); INSIST(ISC_LIST_EMPTY(listener->connections)); - isc_refcount_destroy(&listener->refs); + isc_refcount_destroy(&listener->references); REQUIRE(listener->sock == NULL); @@ -158,18 +187,27 @@ free_listener(controllistener_t *listener) { isc_mem_putanddetach(&listener->mctx, listener, sizeof(*listener)); } -static void -maybe_free_listener(controllistener_t *listener) { - if (isc_refcount_decrement(&listener->refs) == 1) { - free_listener(listener); - } -} +#if NAMED_CONTROLCONF_TRACE +ISC_REFCOUNT_TRACE_IMPL(controllistener, free_listener); +ISC_REFCOUNT_TRACE_IMPL(controlconnection, conn_free); +#else +ISC_REFCOUNT_IMPL(controllistener, free_listener); +ISC_REFCOUNT_IMPL(controlconnection, conn_free); +#endif static void shutdown_listener(controllistener_t *listener) { if (!listener->exiting) { char socktext[ISC_SOCKADDR_FORMATSIZE]; + for (controlconnection_t *conn = + ISC_LIST_HEAD(listener->connections); + conn != NULL; conn = ISC_LIST_HEAD(listener->connections)) + { + control_recvmessage(conn->ccmsg.handle, + ISC_R_SHUTTINGDOWN, conn); + } + ISC_LIST_UNLINK(listener->controls->listeners, listener, link); isc_sockaddr_format(&listener->address, socktext, @@ -188,7 +226,7 @@ shutdown_listener(controllistener_t *listener) { isc_nm_stoplistening(listener->sock); isc_nmsocket_close(&listener->sock); - maybe_free_listener(listener); + controllistener_detach(&listener); } static bool @@ -217,18 +255,12 @@ control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { controllistener_t *listener = conn->listener; isc_sockaddr_t peeraddr = isc_nmhandle_peeraddr(handle); - REQUIRE(conn->sending); - - conn->sending = false; - if (conn->result == ISC_R_SHUTTINGDOWN) { isc_loopmgr_shutdown(named_g_loopmgr); goto cleanup_sendhandle; } - if (atomic_load_acquire(&listener->controls->shuttingdown) || - result == ISC_R_SHUTTINGDOWN) - { + if (listener->controls->shuttingdown || result == ISC_R_SHUTTINGDOWN) { goto cleanup_sendhandle; } else if (result != ISC_R_SUCCESS) { char socktext[ISC_SOCKADDR_FORMATSIZE]; @@ -241,16 +273,13 @@ control_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { goto cleanup_sendhandle; } - isc_nmhandle_attach(handle, &conn->readhandle); - conn->reading = true; - - isc_nmhandle_detach(&conn->sendhandle); - isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, conn); - return; cleanup_sendhandle: - isc_nmhandle_detach(&conn->sendhandle); + if (result != ISC_R_SUCCESS) { + control_recvmessage(handle, result, conn); + } + controlconnection_detach(&conn); } static void @@ -285,7 +314,7 @@ conn_cleanup(controlconnection_t *conn) { } static void -control_respond(isc_nmhandle_t *handle, controlconnection_t *conn) { +control_respond(controlconnection_t *conn) { controllistener_t *listener = conn->listener; isccc_sexpr_t *data = NULL; isc_buffer_t b; @@ -347,7 +376,7 @@ control_respond(isc_nmhandle_t *handle, controlconnection_t *conn) { result = isccc_cc_towire(conn->response, &conn->buffer, conn->alg, &conn->secret); if (result != ISC_R_SUCCESS) { - goto cleanup; + return; } isc_buffer_init(&b, conn->buffer->base, 4); @@ -356,19 +385,11 @@ control_respond(isc_nmhandle_t *handle, controlconnection_t *conn) { r.base = conn->buffer->base; r.length = conn->buffer->used; - isc_nmhandle_attach(handle, &conn->sendhandle); - conn->sending = true; - conn_cleanup(conn); - - isc_nmhandle_detach(&conn->cmdhandle); - - isc_nm_send(conn->sendhandle, &r, control_senddone, conn); - - return; + controlconnection_ref(conn); + isccc_ccmsg_sendmessage(&conn->ccmsg, &r, control_senddone, conn); cleanup: conn_cleanup(conn); - isc_nmhandle_detach(&conn->cmdhandle); } static void @@ -376,19 +397,17 @@ control_command(void *arg) { controlconnection_t *conn = (controlconnection_t *)arg; controllistener_t *listener = conn->listener; - if (atomic_load_acquire(&listener->controls->shuttingdown)) { - conn_cleanup(conn); - isc_nmhandle_detach(&conn->cmdhandle); - return; + if (!listener->controls->shuttingdown) { + conn->result = named_control_docommand( + conn->request, listener->readonly, &conn->text); + control_respond(conn); } - - conn->result = named_control_docommand(conn->request, - listener->readonly, &conn->text); - control_respond(conn->cmdhandle, conn); + controlconnection_detach(&conn); } static void -control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { +control_recvmessage(isc_nmhandle_t *handle ISC_ATTR_UNUSED, isc_result_t result, + void *arg) { controlconnection_t *conn = (controlconnection_t *)arg; controllistener_t *listener = conn->listener; controlkey_t *key = NULL; @@ -396,22 +415,22 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isccc_time_t exp; uint32_t nonce; - conn->reading = false; + if (conn->shuttingdown) { + return; + } /* Is the server shutting down? */ - if (atomic_load_acquire(&listener->controls->shuttingdown)) { - goto cleanup_readhandle; + if (listener->controls->shuttingdown) { + result = ISC_R_SHUTTINGDOWN; } if (result != ISC_R_SUCCESS) { if (result == ISC_R_SHUTTINGDOWN) { - atomic_store_release(&listener->controls->shuttingdown, - true); + listener->controls->shuttingdown = true; } else if (result != ISC_R_EOF) { log_invalid(&conn->ccmsg, result); } - - goto cleanup_readhandle; + goto cleanup; } for (key = ISC_LIST_HEAD(listener->keys); key != NULL; @@ -419,8 +438,7 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { { isccc_region_t ccregion; - ccregion.rstart = isc_buffer_base(conn->ccmsg.buffer); - ccregion.rend = isc_buffer_used(conn->ccmsg.buffer); + isccc_ccmsg_toregion(&conn->ccmsg, &ccregion); conn->secret.rstart = isc_mem_get(listener->mctx, key->secret.length); memmove(conn->secret.rstart, key->secret.base, @@ -507,9 +525,6 @@ control_recvmessage(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isc_buffer_allocate(listener->mctx, &conn->text, 2 * 2048); - isc_nmhandle_attach(handle, &conn->cmdhandle); - isc_nmhandle_detach(&conn->readhandle); - if (conn->nonce == 0) { /* * Establish nonce. @@ -518,45 +533,33 @@ 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, conn); + control_respond(conn); return; } /* * Trigger the command. */ - + controlconnection_ref(conn); isc_async_run(named_g_mainloop, control_command, conn); return; cleanup: - conn_cleanup(conn); - -cleanup_readhandle: - /* - * readhandle could be NULL if we're shutting down, - * but if not we need to detach it. - */ - if (conn->readhandle != NULL) { - isc_nmhandle_detach(&conn->readhandle); - } + conn->shuttingdown = true; + controlconnection_detach(&conn); } static void -conn_reset(void *arg) { - controlconnection_t *conn = (controlconnection_t *)arg; +conn_free(controlconnection_t *conn) { controllistener_t *listener = conn->listener; + conn_cleanup(conn); + if (conn->buffer != NULL) { isc_buffer_free(&conn->buffer); } - if (conn->reading) { - isccc_ccmsg_cancelread(&conn->ccmsg); - return; - } - LOCK(&listener->connections_lock); ISC_LIST_UNLINK(listener->connections, conn, link); UNLOCK(&listener->connections_lock); @@ -567,12 +570,6 @@ conn_reset(void *arg) { #endif /* ifdef ENABLE_AFL */ isccc_ccmsg_invalidate(&conn->ccmsg); -} - -static void -conn_put(void *arg) { - controlconnection_t *conn = (controlconnection_t *)arg; - controllistener_t *listener = conn->listener; isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_CONTROL, ISC_LOG_DEBUG(3), @@ -580,27 +577,22 @@ conn_put(void *arg) { isc_mem_put(listener->mctx, conn, sizeof(*conn)); - maybe_free_listener(listener); + controllistener_detach(&listener); } static void newconnection(controllistener_t *listener, isc_nmhandle_t *handle) { - controlconnection_t *conn = NULL; + controlconnection_t *conn = isc_mem_get(listener->mctx, sizeof(*conn)); + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_CONTROL, ISC_LOG_DEBUG(3), + "allocate new control connection"); - conn = isc_nmhandle_getdata(handle); - if (conn == NULL) { - conn = isc_mem_get(listener->mctx, sizeof(*conn)); - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_CONTROL, ISC_LOG_DEBUG(3), - "allocate new control connection"); - isc_nmhandle_setdata(handle, conn, conn_reset, conn_put); - isc_refcount_increment(&listener->refs); - } + *conn = (controlconnection_t){ .alg = DST_ALG_UNKNOWN }; - *conn = (controlconnection_t){ .listener = listener, - .reading = false, - .alg = DST_ALG_UNKNOWN }; + isc_refcount_init(&conn->references, 1); + controllistener_attach(listener, &conn->listener); + /* isccc_ccmsg_init() attaches to the handle */ isccc_ccmsg_init(listener->mctx, handle, &conn->ccmsg); /* Set a 32 KiB upper limit on incoming message. */ @@ -610,9 +602,6 @@ newconnection(controllistener_t *listener, isc_nmhandle_t *handle) { ISC_LIST_INITANDAPPEND(listener->connections, conn, link); UNLOCK(&listener->connections_lock); - isc_nmhandle_attach(handle, &conn->readhandle); - conn->reading = true; - isccc_ccmsg_readmessage(&conn->ccmsg, control_recvmessage, conn); } @@ -663,7 +652,7 @@ controls_shutdown(named_controls_t *controls) { void named_controls_shutdown(named_controls_t *controls) { controls_shutdown(controls); - atomic_store_release(&controls->shuttingdown, true); + controls->shuttingdown = true; } static isc_result_t @@ -1095,7 +1084,7 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, ISC_LINK_INIT(listener, link); ISC_LIST_INIT(listener->keys); ISC_LIST_INIT(listener->connections); - isc_refcount_init(&listener->refs, 1); + isc_refcount_init(&listener->references, 1); /* * Make the ACL. @@ -1177,7 +1166,7 @@ add_listener(named_controls_t *cp, controllistener_t **listenerp, return; cleanup: - isc_refcount_decrement(&listener->refs); + isc_refcount_decrement(&listener->references); listener->exiting = true; free_listener(listener); @@ -1193,8 +1182,6 @@ cleanup: } *listenerp = NULL; - - /* XXXDCL return error results? fail hard? */ } isc_result_t @@ -1455,7 +1442,6 @@ named_controls_create(named_server_t *server, named_controls_t **ctrlsp) { ISC_LIST_INIT(controls->listeners); - atomic_init(&controls->shuttingdown, false); isc_mutex_init(&controls->symtab_lock); LOCK(&controls->symtab_lock); result = isccc_cc_createsymtab(&controls->symtab); diff --git a/bin/rndc/rndc.c b/bin/rndc/rndc.c index 346c06e4b60..26cfa52e27d 100644 --- a/bin/rndc/rndc.c +++ b/bin/rndc/rndc.c @@ -17,7 +17,6 @@ #include #include -#include #include #include #include @@ -76,18 +75,12 @@ static isccc_region_t secret; static bool failed = false; static bool c_flag = false; static isc_mem_t *rndc_mctx = NULL; -static atomic_uint_fast32_t sends = 0; -static atomic_uint_fast32_t recvs = 0; -static atomic_uint_fast32_t connects = 0; static char *command = NULL; static char *args = NULL; static char program[256]; static uint32_t serial; static bool quiet = false; static bool showresult = false; -static bool shuttingdown = false; -static isc_nmhandle_t *recvdone_handle = NULL; -static isc_nmhandle_t *recvnonce_handle = NULL; static void rndc_startconnect(isc_sockaddr_t *addr); @@ -285,22 +278,11 @@ get_addresses(const char *host, in_port_t port) { } static void -rndc_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { - isc_nmhandle_t *sendhandle = (isc_nmhandle_t *)arg; - +rndc_senddone(isc_nmhandle_t *handle ISC_ATTR_UNUSED, isc_result_t result, + void *arg ISC_ATTR_UNUSED) { if (result != ISC_R_SUCCESS) { fatal("send failed: %s", isc_result_totext(result)); } - - REQUIRE(sendhandle == handle); - isc_nmhandle_detach(&sendhandle); - - if (atomic_fetch_sub_release(&sends, 1) == 1 && - atomic_load_acquire(&recvs) == 0) - { - shuttingdown = true; - isc_loopmgr_shutdown(loopmgr); - } } static void @@ -312,16 +294,10 @@ rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { char *errormsg = NULL; char *textmsg = NULL; + REQUIRE(handle != NULL); REQUIRE(ccmsg != NULL); - if (shuttingdown && (result == ISC_R_EOF || result == ISC_R_CANCELED)) { - atomic_fetch_sub_release(&recvs, 1); - if (handle != NULL) { - REQUIRE(recvdone_handle == handle); - isc_nmhandle_detach(&recvdone_handle); - } - return; - } else if (result == ISC_R_EOF) { + if (result == ISC_R_EOF) { fatal("connection to remote host closed.\n" "* This may indicate that the\n" "* remote server is using an older\n" @@ -377,22 +353,15 @@ rndc_recvdone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isccc_sexpr_free(&response); - REQUIRE(recvdone_handle == handle); - isc_nmhandle_detach(&recvdone_handle); - - if (atomic_fetch_sub_release(&recvs, 1) == 1 && - atomic_load_acquire(&sends) == 0) - { - shuttingdown = true; - isc_loopmgr_shutdown(loopmgr); - } + isccc_ccmsg_invalidate(ccmsg); + isc_loopmgr_shutdown(loopmgr); } static void -rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) { +rndc_recvnonce(isc_nmhandle_t *handle ISC_ATTR_UNUSED, isc_result_t result, + void *arg) { isccc_ccmsg_t *ccmsg = (isccc_ccmsg_t *)arg; isccc_sexpr_t *response = NULL; - isc_nmhandle_t *sendhandle = NULL; isccc_sexpr_t *_ctrl = NULL; isccc_region_t source; uint32_t nonce; @@ -404,14 +373,7 @@ rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) { REQUIRE(ccmsg != NULL); - if (shuttingdown && (result == ISC_R_EOF || result == ISC_R_CANCELED)) { - atomic_fetch_sub_release(&recvs, 1); - if (handle != NULL) { - REQUIRE(recvnonce_handle == handle); - isc_nmhandle_detach(&recvnonce_handle); - } - return; - } else if (result == ISC_R_EOF) { + if (result == ISC_R_EOF) { fatal("connection to remote host closed.\n" "* This may indicate that the\n" "* remote server is using an older\n" @@ -471,17 +433,8 @@ rndc_recvnonce(isc_nmhandle_t *handle, isc_result_t result, void *arg) { r.base = databuf->base; r.length = databuf->used; - isc_nmhandle_attach(handle, &recvdone_handle); - atomic_fetch_add_relaxed(&recvs, 1); isccc_ccmsg_readmessage(ccmsg, rndc_recvdone, ccmsg); - - isc_nmhandle_attach(handle, &sendhandle); - atomic_fetch_add_relaxed(&sends, 1); - isc_nm_send(handle, &r, rndc_senddone, sendhandle); - - REQUIRE(recvnonce_handle == handle); - isc_nmhandle_detach(&recvnonce_handle); - atomic_fetch_sub_release(&recvs, 1); + isccc_ccmsg_sendmessage(ccmsg, &r, rndc_senddone, NULL); isccc_sexpr_free(&response); isccc_sexpr_free(&request); @@ -497,13 +450,10 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isccc_time_t now = isc_stdtime_now(); isc_region_t r; isc_buffer_t b; - isc_nmhandle_t *connhandle = NULL; - isc_nmhandle_t *sendhandle = NULL; REQUIRE(ccmsg != NULL); if (result != ISC_R_SUCCESS) { - atomic_fetch_sub_release(&connects, 1); isc_sockaddr_format(&serveraddrs[currentaddr], socktext, sizeof(socktext)); if (++currentaddr < nserveraddrs) { @@ -517,8 +467,6 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) { isc_result_totext(result)); } - isc_nmhandle_attach(handle, &connhandle); - DO("create message", isccc_cc_createmessage(1, NULL, NULL, ++serial, now, now + 60, &request)); data = isccc_alist_lookup(request, "_data"); @@ -542,19 +490,12 @@ rndc_connected(isc_nmhandle_t *handle, isc_result_t result, void *arg) { r.base = databuf->base; r.length = databuf->used; + /* isccc_ccmsg_init() attaches to the handle */ isccc_ccmsg_init(rndc_mctx, handle, ccmsg); isccc_ccmsg_setmaxsize(ccmsg, 1024 * 1024); - isc_nmhandle_attach(handle, &recvnonce_handle); - atomic_fetch_add_relaxed(&recvs, 1); isccc_ccmsg_readmessage(ccmsg, rndc_recvnonce, ccmsg); - - isc_nmhandle_attach(handle, &sendhandle); - atomic_fetch_add_relaxed(&sends, 1); - isc_nm_send(handle, &r, rndc_senddone, sendhandle); - - isc_nmhandle_detach(&connhandle); - atomic_fetch_sub_release(&connects, 1); + isccc_ccmsg_sendmessage(ccmsg, &r, rndc_senddone, NULL); isccc_sexpr_free(&request); } @@ -584,7 +525,6 @@ rndc_startconnect(isc_sockaddr_t *addr) { UNREACHABLE(); } - atomic_fetch_add_relaxed(&connects, 1); isc_nm_tcpconnect(netmgr, local, addr, rndc_connected, &rndc_ccmsg, 60000); } @@ -1062,14 +1002,6 @@ main(int argc, char **argv) { isc_loopmgr_run(loopmgr); - /* - * Note: when TCP connections are shut down, there will be a final - * call to the isccc callback routine with &rndc_ccmsg as its - * argument. We therefore need to delay invalidating it until - * after the netmgr is closed down. - */ - isccc_ccmsg_invalidate(&rndc_ccmsg); - isc_log_destroy(&log); isc_log_setcontext(NULL); diff --git a/doc/design/netmgr.md b/doc/design/netmgr.md index b62e8dd913e..a5f34f3f29a 100644 --- a/doc/design/netmgr.md +++ b/doc/design/netmgr.md @@ -79,31 +79,103 @@ inactive-handles stack is full or when the socket is destroyed) then the associated object's 'put' callback will be called to free any resources it allocated. -## UDP listening +## Streaming Protocols -UDP listener sockets automatically create an array of 'child' sockets, -each associated with one networker, and all listening on the same address -via `SO_REUSEADDR`. (The parent's reference counter is used for all the -parent and child sockets together; none are destroyed until there are no -remaining references to any of tem.) +Currently, we have two streaming protocols available in Network Manager - TCP +and TLS. The underlying premise is that they both expose the same interface to +the clients. -## TCP listening +### Servers (Listening) -A TCP listener socket cannot listen on multiple threads in parallel, -so receiving a TCP connection can cause a context switch, but this is -expected to be rare enough not to impact performance significantly. +The users of the API calls ``isc_nm_listentcp()`` or ``isc_nm_listentls()`` with +the accept callback as argument. -When connected, a TCP socket will attach to the system-wide TCP clients -quota. +When connection is accepted, the accept callback is called with a handle and +status and it can return a non-``ISC_R_RESULT`` to abort the connection. -## TCP listening for DNS +The accept callback should generally immediately call ``isc_nm_read()`` to setup +the read callback. Not doing so, can lead to a data race - if the NM is shut +down before the ``isc_nm_read()`` call, the socket can become dangling until +``isc_nm_read()`` is finally called. -A TCPDNS listener is a wrapper around a TCP socket which specifically -handles DNS traffic, including the two-byte length field that prepends DNS -messages over TCP. +When ``isc_nm_read()`` is called, the read callback will receive: -Other wrapper socket types can be added in the future, such as a TLS socket -wrapper to implement encryption or an HTTP wrapper to implement the HTTP -protocol. This will enable the system to have a transport-neutral network -manager socket over which DNS can be sent without knowing anything about -transport, encryption, etc. +- 0- calls with ``ISC_R_SUCCESS`` state +- exactly 1 call with non-``ISC_R_SUCCESS`` state when the connection is + interrupted (locally closed, remotely closed, NM shutting down, etc.) + +The ``isc_nm_read_stop()`` can be used to pause reading from the socket and only +the final non-``ISC_R_SUCCESS`` callback will be received in such case. + +### Clients (Connecting) + +The users of the API calls ``isc_nm_tcpconnect()`` or ``isc_nm_tlsconnect()`` +with the connect callback as argument. + +When connection is established, the connect callback is called with a handle and +status. + +The connect callback should generally immediately call ``isc_nm_read()`` - see +the same caveat in the accepting part. + +When ``isc__nm_read()`` is called on the connected socket, the read callback +will receive: + +- 0- calls with ``ISC_R_SUCCESS`` state +- exactly 1 call with non-``ISC_R_SUCCESS`` state when the connection is + interrupted (locally closed, remotely closed, NM shutting down, etc.) + +The ``isc_nm_read_stop()`` can be used to pause reading from the socket and only +the final non-``ISC_R_SUCCESS`` callback will be received in such case. + +## DNS Message Protocols + +Currently, we have three (four) DNS Message Protocols implemented in the Network Manager: + +- UDP +- StreamDNS (TCPDNS and TLSDNS) +- HTTP + +### Servers (Listening) + +The users of the API calls ``isc_nm_listenudp()`` or +``isc_nm_listenstreamdns()`` with: + +- accept callback +- read callback + +The StreamDNS accepts an optional TLS context for DoT (otherwise DNS over TCP +will be used). + +The HTTP listening is more complicated - the users need to setup the endpoints +with the read callback and pass the 1- endpoints to the +``isc_nm_listenhttp()`` call. + +The accept callback is used only to implement "firewall"-like functionality, it +could be used to tear down the connection early in the process. + +After the connection has been accepted, the read callback will receive: + +- 0- calls with ``ISC_R_SUCCESS`` state +- exactly 1 call with non-``ISC_R_SUCCESS`` state when the connection is + interrupted (locally closed, remotely closed, NM shutting down, etc.) + +Each read callback will contain a full assembled DNS message. + +### Clients (Connecting) + +The users of the API calls ``isc_nm_udpconnect()``, +``isc_nm_streamdnsconnect()``, or ``isc_nm_httpconnect()`` with a connect +callback. + +When connection is established, the connect callback is called with a handle and +status. + +The connect callback should generally immediately call ``isc_nm_read()`` - see +the caveat in the previous parts. + +After the connection has been connected, the read callback will receive exactly +1 call for each ``isc_nm_read()`` call - either with ``ISC_R_SUCCESS`` if the +DNS message was successfully read or non-``ISC_R_SUCCESS`` indicating the error +condition. The read callback either needs to issue new ``isc_nm_read()`` call +or detach from the handle if no further messages are required. diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index 5e81de0b970..ef92a3e093e 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -92,8 +92,7 @@ struct isc_httpd { isc_httpdmgr_t *mgr; /*%< our parent */ ISC_LINK(isc_httpd_t) link; - isc_nmhandle_t *handle; /* Permanent pointer to handle */ - isc_nmhandle_t *readhandle; /* Waiting for a read callback */ + isc_nmhandle_t *handle; /* Permanent pointer to handle */ int flags; @@ -134,7 +133,6 @@ struct isc_httpdmgr { typedef struct isc_httpd_sendreq { isc_mem_t *mctx; isc_httpd_t *httpd; - isc_nmhandle_t *handle; /*% * Transmit data state. @@ -178,9 +176,7 @@ httpd_request(isc_nmhandle_t *, isc_result_t, isc_region_t *, void *); static void httpd_senddone(isc_nmhandle_t *, isc_result_t, void *); static void -httpd_reset(void *); -static void -httpd_put(void *); +httpd_free(isc_httpd_t *httpd); static void httpd_addheader(isc_httpd_sendreq_t *, const char *, const char *); @@ -509,8 +505,7 @@ process_request(isc_httpd_t *httpd, size_t last_len) { } static void -httpd_reset(void *arg) { - isc_httpd_t *httpd = (isc_httpd_t *)arg; +httpd_free(isc_httpd_t *httpd) { isc_httpdmgr_t *httpdmgr = NULL; REQUIRE(VALID_HTTPD(httpd)); @@ -533,6 +528,19 @@ httpd_reset(void *arg) { httpd->path = NULL; httpd->up = (isc_url_parser_t){ 0 }; isc_time_set(&httpd->if_modified_since, 0, 0); + + httpd->magic = 0; + httpd->mgr = NULL; + + isc_mem_put(httpdmgr->mctx, httpd, sizeof(*httpd)); + + httpdmgr_detach(&httpdmgr); + +#if ENABLE_AFL + if (finishhook != NULL) { + finishhook(); + } +#endif /* ENABLE_AFL */ } static void @@ -567,60 +575,27 @@ isc__httpd_sendreq_new(isc_httpd_t *httpd) { return (req); } -static void -httpd_put(void *arg) { - isc_httpd_t *httpd = (isc_httpd_t *)arg; - isc_httpdmgr_t *mgr = NULL; - - REQUIRE(VALID_HTTPD(httpd)); - - mgr = httpd->mgr; - REQUIRE(VALID_HTTPDMGR(mgr)); - - httpd->magic = 0; - httpd->mgr = NULL; - - isc_mem_put(mgr->mctx, httpd, sizeof(*httpd)); - - httpdmgr_detach(&mgr); - -#if ENABLE_AFL - if (finishhook != NULL) { - finishhook(); - } -#endif /* ENABLE_AFL */ -} - static void new_httpd(isc_httpdmgr_t *httpdmgr, isc_nmhandle_t *handle) { isc_httpd_t *httpd = NULL; REQUIRE(VALID_HTTPDMGR(httpdmgr)); - httpd = isc_nmhandle_getdata(handle); - if (httpd == NULL) { - httpd = isc_mem_get(httpdmgr->mctx, sizeof(*httpd)); - *httpd = (isc_httpd_t){ .handle = NULL }; - httpdmgr_attach(httpdmgr, &httpd->mgr); - } - - if (httpd->handle == NULL) { - isc_nmhandle_setdata(handle, httpd, httpd_reset, httpd_put); - httpd->handle = handle; - } else { - INSIST(httpd->handle == handle); - } + httpd = isc_mem_get(httpdmgr->mctx, sizeof(*httpd)); + *httpd = (isc_httpd_t){ + .magic = HTTPD_MAGIC, + .link = ISC_LINK_INITIALIZER, + }; - ISC_LINK_INIT(httpd, link); + isc_nmhandle_attach(handle, &httpd->handle); - httpd->magic = HTTPD_MAGIC; + httpdmgr_attach(httpdmgr, &httpd->mgr); LOCK(&httpdmgr->lock); ISC_LIST_APPEND(httpdmgr->running, httpd, link); UNLOCK(&httpdmgr->lock); - isc_nmhandle_attach(httpd->handle, &httpd->readhandle); - isc_nm_read(handle, httpd_request, httpdmgr); + isc_nm_read(handle, httpd_request, httpd); } static isc_result_t @@ -885,14 +860,12 @@ prepare_response(isc_httpdmgr_t *mgr, isc_httpd_t *httpd, static void httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, void *arg) { - isc_result_t result; - isc_httpdmgr_t *mgr = arg; - isc_httpd_t *httpd = NULL; + isc_httpd_t *httpd = arg; + isc_httpdmgr_t *mgr = httpd->mgr; isc_httpd_sendreq_t *req = NULL; isc_region_t r; size_t last_len = 0; - - httpd = isc_nmhandle_getdata(handle); + isc_result_t result; REQUIRE(VALID_HTTPD(httpd)); @@ -902,10 +875,9 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, goto close_readhandle; } - REQUIRE(httpd->readhandle == handle); REQUIRE((mgr->flags & ISC_HTTPDMGR_SHUTTINGDOWN) == 0); - isc_nm_read_stop(httpd->readhandle); + isc_nm_read_stop(handle); /* * If we are being called from httpd_senddone(), the last HTTP request @@ -932,7 +904,7 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, goto close_readhandle; } - /* Wait for more data, the readhandle is still attached */ + /* Wait for more data, the handle is still attached */ isc_nm_read(handle, httpd_request, arg); return; } @@ -948,16 +920,15 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, */ isc_buffer_usedregion(req->sendbuffer, &r); - isc_nmhandle_attach(httpd->handle, &req->handle); - isc_nm_send(httpd->handle, &r, httpd_senddone, req); - - isc_nmhandle_detach(&httpd->readhandle); + isc_nmhandle_ref(handle); + isc_nm_send(handle, &r, httpd_senddone, req); return; close_readhandle: - isc_nm_read_stop(httpd->readhandle); - isc_nmhandle_close(httpd->readhandle); - isc_nmhandle_detach(&httpd->readhandle); + isc_nmhandle_close(httpd->handle); + isc_nmhandle_detach(&httpd->handle); + + httpd_free(httpd); } void @@ -978,9 +949,9 @@ isc_httpdmgr_shutdown(isc_httpdmgr_t **httpdmgrp) { httpd = ISC_LIST_HEAD(httpdmgr->running); while (httpd != NULL) { - if (httpd->readhandle != NULL) { - httpd_request(httpd->readhandle, ISC_R_SHUTTINGDOWN, - NULL, httpdmgr); + if (httpd->handle != NULL) { + httpd_request(httpd->handle, ISC_R_SUCCESS, NULL, + httpd); } httpd = ISC_LIST_NEXT(httpd, link); } @@ -1045,17 +1016,16 @@ httpd_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { goto detach; } - if (eresult == ISC_R_SUCCESS && (httpd->flags & HTTPD_CLOSE) == 0) { - /* - * Calling httpd_request() with region NULL restarts - * reading. - */ - isc_nmhandle_attach(handle, &httpd->readhandle); - httpd_request(handle, ISC_R_SUCCESS, NULL, httpd->mgr); - } else { - isc_nmhandle_close(handle); + if (eresult == ISC_R_SUCCESS && (httpd->flags & HTTPD_CLOSE) != 0) { + eresult = ISC_R_EOF; } + /* + * Calling httpd_request() with region NULL restarts + * reading. + */ + httpd_request(handle, eresult, NULL, httpd); + detach: isc_nmhandle_detach(&handle); isc__httpd_sendreq_free(req); diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 944dccaabb8..19250b52830 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -148,25 +149,18 @@ isc_nmsocket_set_max_streams(isc_nmsocket_t *listener, */ #if ISC_NETMGR_TRACE -#define isc_nmhandle_attach(handle, dest) \ - isc__nmhandle_attach(handle, dest, __FILE__, __LINE__, __func__) -#define isc_nmhandle_detach(handlep) \ - isc__nmhandle_detach(handlep, __FILE__, __LINE__, __func__) -#define FLARG_PASS , file, line, func -#define FLARG \ - , const char *file ISC_ATTR_UNUSED, unsigned int line ISC_ATTR_UNUSED, \ - const char *func ISC_ATTR_UNUSED +#define isc_nmhandle_ref(ptr) \ + isc_nmhandle__ref(ptr, __func__, __FILE__, __LINE__) +#define isc_nmhandle_unref(ptr) \ + isc_nmhandle__unref(ptr, __func__, __FILE__, __LINE__) +#define isc_nmhandle_attach(ptr, ptrp) \ + isc_nmhandle__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define isc_nmhandle_detach(ptrp) \ + isc_nmhandle__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(isc_nmhandle); #else -#define isc_nmhandle_attach(handle, dest) isc__nmhandle_attach(handle, dest) -#define isc_nmhandle_detach(handlep) isc__nmhandle_detach(handlep) -#define FLARG_PASS -#define FLARG +ISC_REFCOUNT_DECL(isc_nmhandle); #endif - -void -isc__nmhandle_attach(isc_nmhandle_t *handle, isc_nmhandle_t **dest FLARG); -void -isc__nmhandle_detach(isc_nmhandle_t **handlep FLARG); /*%< * Increment/decrement the reference counter in a netmgr handle. * @@ -175,7 +169,6 @@ isc__nmhandle_detach(isc_nmhandle_t **handlep FLARG); * event loop. When references go to zero, the associated socket will be * closed and deleted. */ -#undef FLARG void * isc_nmhandle_getdata(isc_nmhandle_t *handle); @@ -310,12 +303,18 @@ isc_nm_cancelread(isc_nmhandle_t *handle); * active handles with a result code of ISC_R_CANCELED. * * Requires: - * \li 'sock' is a valid datagram-like netmgr socket + * \li 'handle' is a valid netmgr handle * \li ...for which a read/recv callback has been defined. */ void isc_nmhandle_close(isc_nmhandle_t *handle); +/*%< + * Close the active handle - no further read callbacks will happen. + * + * Requires: + * 'li 'handle' is a valid netmgr handle + */ void isc_nm_send(isc_nmhandle_t *handle, isc_region_t *region, isc_nm_cb_t cb, @@ -349,7 +348,8 @@ isc_nm_listentcp(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface, void isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, - isc_nm_cb_t cb, void *cbarg, unsigned int timeout); + isc_nm_cb_t connect_cb, void *connect_cbarg, + unsigned int timeout); /*%< * Create a socket using netmgr 'mgr', bind it to the address 'local', * and connect it to the address 'peer'. @@ -507,7 +507,8 @@ isc_nm_listentls(isc_nm_t *mgr, uint32_t workers, isc_sockaddr_t *iface, void isc_nm_tlsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, - isc_nm_cb_t cb, void *cbarg, isc_tlsctx_t *ctx, + isc_nm_cb_t connect_cb, void *connect_cbarg, + isc_tlsctx_t *ctx, isc_tlsctx_client_session_cache_t *client_sess_cache, unsigned int timeout); diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index 7ba075fe907..e3617d8e69a 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -975,15 +975,13 @@ client_submit_request(isc_nm_http_session_t *session, http_cstream_t *stream) { * Read callback from TLS socket. */ static void -http_readcb(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region, - void *data) { +http_readcb(isc_nmhandle_t *handle ISC_ATTR_UNUSED, isc_result_t result, + isc_region_t *region, void *data) { isc_nm_http_session_t *session = (isc_nm_http_session_t *)data; ssize_t readlen; REQUIRE(VALID_HTTP2_SESSION(session)); - UNUSED(handle); - if (result != ISC_R_SUCCESS) { if (result != ISC_R_TIMEDOUT) { session->reading = false; diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index bf90e1512a6..e3931d71fb2 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -115,53 +115,58 @@ STATIC_ASSERT(ISC_NETMGR_TCP_RECVBUF_SIZE <= ISC_NETMGR_RECVBUF_SIZE, #if defined(__linux__) #include -#define gettid() (uint32_t) syscall(SYS_gettid) +#define gettid() (uint64_t) syscall(SYS_gettid) +#elif defined(__FreeBSD__) +#include +#define gettid() (uint64_t)(pthread_getthreadid_np()) +#elif defined(__OpenBSD__) +#include +#define gettid() (uint64_t)(getthrid()) +#elif defined(__NetBSD__) +#include +#define gettid() (uint64_t)(_lwp_self()) +#elif defined(__DragonFly__) +#include +#define gettid() (uint64_t)(lwp_gettid()) #else -#define gettid() (uint32_t) pthread_self() +#define gettid() (uint64_t)(pthread_self()) #endif #define NETMGR_TRACE_LOG(format, ...) \ - fprintf(stderr, "%" PRIu32 ":%d:%s:%u:%s:" format, gettid(), \ + fprintf(stderr, "%" PRIu64 ":%d:%s:%u:%s:" format, gettid(), \ isc_tid(), file, line, func, __VA_ARGS__) -#define FLARG \ - , const char *file ISC_ATTR_UNUSED, unsigned int line ISC_ATTR_UNUSED, \ - const char *func ISC_ATTR_UNUSED -#define FLARG_PASS , file, line, func -#define FLARG_IEVENT(ievent) \ - const char *file = ievent->file; \ - unsigned int line = ievent->line; \ - const char *func = ievent->func; -#define FLARG_IEVENT_PASS(ievent) \ - ievent->file = file; \ - ievent->line = line; \ - ievent->func = func; +#define FLARG \ + , const char *func ISC_ATTR_UNUSED, const char *file ISC_ATTR_UNUSED, \ + unsigned int line ISC_ATTR_UNUSED + +#define FLARG_PASS , func, file, line #define isc__nm_uvreq_get(sock) \ - isc___nm_uvreq_get(sock, __FILE__, __LINE__, __func__) + isc___nm_uvreq_get(sock, __func__, __FILE__, __LINE__) #define isc__nm_uvreq_put(req) \ - isc___nm_uvreq_put(req, __FILE__, __LINE__, __func__) + isc___nm_uvreq_put(req, __func__, __FILE__, __LINE__) #define isc__nmsocket_init(sock, mgr, type, iface, parent) \ - isc___nmsocket_init(sock, mgr, type, iface, parent, __FILE__, \ - __LINE__, __func__) + isc___nmsocket_init(sock, mgr, type, iface, parent, __func__, \ + __FILE__, __LINE__) #define isc__nmsocket_put(sockp) \ - isc___nmsocket_put(sockp, __FILE__, __LINE__, __func__) + isc___nmsocket_put(sockp, __func__, __FILE__, __LINE__) #define isc__nmsocket_attach(sock, target) \ - isc___nmsocket_attach(sock, target, __FILE__, __LINE__, __func__) + isc___nmsocket_attach(sock, target, __func__, __FILE__, __LINE__) #define isc__nmsocket_detach(socketp) \ - isc___nmsocket_detach(socketp, __FILE__, __LINE__, __func__) + isc___nmsocket_detach(socketp, __func__, __FILE__, __LINE__) #define isc__nmsocket_close(socketp) \ - isc___nmsocket_close(socketp, __FILE__, __LINE__, __func__) + isc___nmsocket_close(socketp, __func__, __FILE__, __LINE__) #define isc__nmhandle_get(sock, peer, local) \ - isc___nmhandle_get(sock, peer, local, __FILE__, __LINE__, __func__) + isc___nmhandle_get(sock, peer, local, __func__, __FILE__, __LINE__) #define isc__nmsocket_prep_destroy(sock) \ - isc___nmsocket_prep_destroy(sock, __FILE__, __LINE__, __func__) + isc___nmsocket_prep_destroy(sock, __func__, __FILE__, __LINE__) +#define isc__nm_get_read_req(sock, sockaddr) \ + isc___nm_get_read_req(sock, sockaddr, __func__, __FILE__, __LINE__) #else #define NETMGR_TRACE_LOG(format, ...) #define FLARG #define FLARG_PASS -#define FLARG_IEVENT(ievent) -#define FLARG_IEVENT_PASS(ievent) #define isc__nm_uvreq_get(sock) isc___nm_uvreq_get(sock) #define isc__nm_uvreq_put(req) isc___nm_uvreq_put(req) #define isc__nmsocket_init(sock, mgr, type, iface, parent) \ @@ -173,6 +178,8 @@ STATIC_ASSERT(ISC_NETMGR_TCP_RECVBUF_SIZE <= ISC_NETMGR_RECVBUF_SIZE, #define isc__nmhandle_get(sock, peer, local) \ isc___nmhandle_get(sock, peer, local) #define isc__nmsocket_prep_destroy(sock) isc___nmsocket_prep_destroy(sock) +#define isc__nm_get_read_req(sock, sockaddr) \ + isc___nm_get_read_req(sock, sockaddr) #endif typedef struct isc__nm_uvreq isc__nm_uvreq_t; @@ -504,6 +511,7 @@ struct isc_nmsocket { size_t nsending; bool tcp_nodelay_value; isc_nmsocket_tls_send_req_t *send_req; /*%< Send req to reuse */ + bool reading; } tlsstream; #if HAVE_LIBNGHTTP2 @@ -637,7 +645,6 @@ struct isc_nmsocket { isc_nmhandle_t *recv_handle; isc_nm_recv_cb_t recv_cb; void *recv_cbarg; - bool recv_read; isc_nm_cb_t connect_cb; void *connect_cbarg; @@ -806,12 +813,6 @@ isc__nm_udp_close(isc_nmsocket_t *sock); * Close a UDP socket. */ -void -isc__nm_udp_cancelread(isc_nmhandle_t *handle); -/*%< - * Stop reading on a connected UDP handle. - */ - void isc__nm_udp_shutdown(isc_nmsocket_t *sock); /*%< @@ -862,12 +863,6 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock); * sockets. */ -void -isc__nm_tcp_cancelread(isc_nmhandle_t *handle); -/*%< - * Stop reading on a connected TCP handle. - */ - void isc__nm_tcp_stoplistening(isc_nmsocket_t *sock); /*%< @@ -1089,9 +1084,6 @@ isc__nm_streamdns_stoplistening(isc_nmsocket_t *sock); void isc__nm_streamdns_cleanup_data(isc_nmsocket_t *sock); -void -isc__nm_streamdns_cancelread(isc_nmhandle_t *handle); - void isc__nmhandle_streamdns_cleartimeout(isc_nmhandle_t *handle); @@ -1249,7 +1241,7 @@ isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result, bool async); isc__nm_uvreq_t * -isc__nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr); +isc___nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr FLARG); void isc__nm_alloc_cb(uv_handle_t *handle, size_t size, uv_buf_t *buf); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 43610b092c9..583febaa8a5 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -580,15 +580,6 @@ nmsocket_maybe_destroy(isc_nmsocket_t *sock FLARG) { } } -void -isc_nmhandle_close(isc_nmhandle_t *handle) { - REQUIRE(VALID_NMHANDLE(handle)); - REQUIRE(VALID_NMSOCK(handle->sock)); - - isc__nmsocket_clearcb(handle->sock); - isc__nm_failed_read_cb(handle->sock, ISC_R_EOF, false); -} - void isc___nmsocket_prep_destroy(isc_nmsocket_t *sock FLARG) { REQUIRE(sock->parent == NULL); @@ -904,19 +895,6 @@ isc___nmhandle_get(isc_nmsocket_t *sock, isc_sockaddr_t const *peer, return (handle); } -void -isc__nmhandle_attach(isc_nmhandle_t *handle, isc_nmhandle_t **handlep FLARG) { - REQUIRE(VALID_NMHANDLE(handle)); - REQUIRE(handlep != NULL && *handlep == NULL); - - NETMGR_TRACE_LOG("isc__nmhandle_attach():handle %p->references = " - "%" PRIuFAST32 "\n", - handle, isc_refcount_current(&handle->references) + 1); - - isc_refcount_increment(&handle->references); - *handlep = handle; -} - bool isc_nmhandle_is_stream(isc_nmhandle_t *handle) { REQUIRE(VALID_NMHANDLE(handle)); @@ -1003,26 +981,11 @@ nmhandle_destroy(isc_nmhandle_t *handle) { handle); } -void -isc__nmhandle_detach(isc_nmhandle_t **handlep FLARG) { - isc_nmhandle_t *handle = NULL; - - REQUIRE(handlep != NULL); - REQUIRE(VALID_NMHANDLE(*handlep)); - - handle = *handlep; - *handlep = NULL; - - REQUIRE(handle->sock->tid == isc_tid()); - - NETMGR_TRACE_LOG("isc__nmhandle_detach():%p->references = %" PRIuFAST32 - "\n", - handle, isc_refcount_current(&handle->references) - 1); - - if (isc_refcount_decrement(&handle->references) == 1) { - nmhandle_destroy(handle); - } -} +#if ISC_NETMGR_TRACE +ISC_REFCOUNT_TRACE_IMPL(isc_nmhandle, nmhandle_destroy) +#else +ISC_REFCOUNT_IMPL(isc_nmhandle, nmhandle_destroy); +#endif void * isc_nmhandle_getdata(isc_nmhandle_t *handle) { @@ -1117,7 +1080,6 @@ isc__nmsocket_connecttimeout_cb(uv_timer_t *timer) { */ REQUIRE(!sock->timedout); sock->timedout = true; - isc__nmsocket_clearcb(sock); isc__nmsocket_shutdown(sock); } @@ -1168,13 +1130,10 @@ isc__nmsocket_readtimeout_cb(uv_timer_t *timer) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_tid()); - REQUIRE(sock->reading); if (sock->client) { uv_timer_stop(timer); - sock->recv_read = false; - if (sock->recv_cb != NULL) { isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL); isc__nm_readcb(sock, req, ISC_R_TIMEDOUT, false); @@ -1182,7 +1141,7 @@ isc__nmsocket_readtimeout_cb(uv_timer_t *timer) { if (!isc__nmsocket_timer_running(sock)) { isc__nmsocket_clearcb(sock); - isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false); + isc__nm_failed_read_cb(sock, ISC_R_TIMEDOUT, false); } } else { isc__nm_failed_read_cb(sock, ISC_R_TIMEDOUT, false); @@ -1285,7 +1244,7 @@ isc__nmsocket_timer_stop(isc_nmsocket_t *sock) { } isc__nm_uvreq_t * -isc__nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr) { +isc___nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr FLARG) { isc__nm_uvreq_t *req = NULL; req = isc__nm_uvreq_get(sock); @@ -1295,16 +1254,32 @@ isc__nm_get_read_req(isc_nmsocket_t *sock, isc_sockaddr_t *sockaddr) { switch (sock->type) { case isc_nm_tcpsocket: case isc_nm_tlssocket: +#if ISC_NETMGR_TRACE + isc_nmhandle__attach(sock->statichandle, + &req->handle FLARG_PASS); +#else isc_nmhandle_attach(sock->statichandle, &req->handle); +#endif break; case isc_nm_streamdnssocket: +#if ISC_NETMGR_TRACE + isc_nmhandle__attach(sock->recv_handle, + &req->handle FLARG_PASS); +#else isc_nmhandle_attach(sock->recv_handle, &req->handle); +#endif break; default: if (sock->client && sock->statichandle != NULL) { +#if ISC_NETMGR_TRACE + isc_nmhandle__attach(sock->statichandle, + &req->handle FLARG_PASS); +#else isc_nmhandle_attach(sock->statichandle, &req->handle); +#endif } else { - req->handle = isc__nmhandle_get(sock, sockaddr, NULL); + req->handle = isc___nmhandle_get(sock, sockaddr, + NULL FLARG_PASS); } break; } @@ -1357,7 +1332,7 @@ isc__nm_start_reading(isc_nmsocket_t *sock) { isc_result_t result = ISC_R_SUCCESS; int r; - if (sock->reading) { + if (uv_is_active(&sock->uv_handle.handle)) { return (ISC_R_SUCCESS); } @@ -1375,8 +1350,6 @@ isc__nm_start_reading(isc_nmsocket_t *sock) { } if (r != 0) { result = isc_uverr2result(r); - } else { - sock->reading = true; } return (result); @@ -1386,7 +1359,7 @@ void isc__nm_stop_reading(isc_nmsocket_t *sock) { int r; - if (!sock->reading) { + if (!uv_is_active(&sock->uv_handle.handle)) { return; } @@ -1402,7 +1375,6 @@ isc__nm_stop_reading(isc_nmsocket_t *sock) { default: UNREACHABLE(); } - sock->reading = false; } bool @@ -1578,7 +1550,11 @@ isc___nm_uvreq_put(isc__nm_uvreq_t **reqp FLARG) { ISC_LIST_UNLINK(sock->active_uvreqs, req, active_link); if (handle != NULL) { - isc__nmhandle_detach(&handle FLARG_PASS); +#if ISC_NETMGR_TRACE + isc_nmhandle__detach(&handle, func, file, line); +#else + isc_nmhandle_detach(&handle); +#endif } isc_mempool_put(sock->worker->uvreq_pool, req); @@ -1659,20 +1635,37 @@ isc_nm_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) { } } -void -isc_nm_cancelread(isc_nmhandle_t *handle) { +static void +cancelread_cb(void *arg) { + isc_nmhandle_t *handle = arg; + REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(VALID_NMSOCK(handle->sock)); + REQUIRE(handle->sock->tid == isc_tid()); + + REQUIRE(handle->sock->tid == isc_tid()); switch (handle->sock->type) { case isc_nm_udpsocket: - isc__nm_udp_cancelread(handle); - break; case isc_nm_streamdnssocket: - isc__nm_streamdns_cancelread(handle); + case isc_nm_httpsocket: + isc__nm_failed_read_cb(handle->sock, ISC_R_CANCELED, false); break; default: UNREACHABLE(); } + + isc_nmhandle_detach(&handle); +} + +void +isc_nm_cancelread(isc_nmhandle_t *handle) { + REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(VALID_NMSOCK(handle->sock)); + + /* Running this directly could cause a dead-lock */ + isc_nmhandle_ref(handle); + isc_async_run(handle->sock->worker->loop, cancelread_cb, handle); } void @@ -1693,6 +1686,15 @@ isc_nm_read_stop(isc_nmhandle_t *handle) { } } +void +isc_nmhandle_close(isc_nmhandle_t *handle) { + REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(VALID_NMSOCK(handle->sock)); + + isc__nmsocket_clearcb(handle->sock); + isc__nmsocket_prep_destroy(handle->sock); +} + void isc_nm_stoplistening(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); @@ -2505,8 +2507,7 @@ nmsocket_dump(isc_nmsocket_t *sock) { "Parent %p, listener %p, server %p, statichandle = " "%p\n", sock->parent, sock->listener, sock->server, sock->statichandle); - fprintf(stderr, "Flags:%s%s%s%s%s\n", - atomic_load_acquire(&sock->active) ? " active" : "", + fprintf(stderr, "Flags:%s%s%s%s%s\n", sock->active ? " active" : "", sock->closing ? " closing" : "", sock->destroying ? " destroying" : "", sock->connecting ? " connecting" : "", diff --git a/lib/isc/netmgr/streamdns.c b/lib/isc/netmgr/streamdns.c index 2f1c5a3bb10..64b20ff735e 100644 --- a/lib/isc/netmgr/streamdns.c +++ b/lib/isc/netmgr/streamdns.c @@ -131,7 +131,7 @@ streamdns_on_complete_dnsmessage(isc_dnsstream_assembler_t *dnsasm, */ bool stop = sock->client; - sock->recv_read = false; + sock->reading = false; if (sock->recv_cb != NULL) { if (!sock->client) { /* @@ -271,9 +271,7 @@ static void streamdns_call_connect_cb(isc_nmsocket_t *sock, isc_nmhandle_t *handle, const isc_result_t result) { sock->connecting = false; - if (sock->connect_cb == NULL) { - return; - } + INSIST(sock->connect_cb != NULL); sock->connect_cb(handle, result, sock->connect_cbarg); if (result != ISC_R_SUCCESS) { isc__nmsocket_clearcb(handle->sock); @@ -401,12 +399,6 @@ isc_nm_streamdnsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, } } -static bool -streamdns_waiting_for_msg(isc_nmsocket_t *sock) { - /* There is an unsatisfied read operation pending */ - return (sock->recv_read); -} - bool isc__nmsocket_streamdns_timer_running(isc_nmsocket_t *sock) { isc_nmsocket_t *transp_sock; @@ -464,36 +456,45 @@ isc__nmsocket_streamdns_timer_restart(isc_nmsocket_t *sock) { static void streamdns_failed_read_cb(isc_nmsocket_t *sock, const isc_result_t result, const bool async) { - bool destroy = true; - REQUIRE(VALID_NMSOCK(sock)); REQUIRE(result != ISC_R_SUCCESS); - if (sock->recv_cb != NULL && sock->recv_handle != NULL && - (streamdns_waiting_for_msg(sock) || result == ISC_R_TIMEDOUT)) - { - isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL); - - INSIST(VALID_NMHANDLE(sock->recv_handle)); + /* Nobody is reading from the socket yet */ + if (sock->recv_handle == NULL) { + goto destroy; + } - if (result != ISC_R_TIMEDOUT) { - sock->recv_read = false; - isc_dnsstream_assembler_clear(sock->streamdns.input); - isc__nmsocket_clearcb(sock); - } else if (sock->client) { - sock->recv_read = false; + if (sock->client && result == ISC_R_TIMEDOUT) { + if (sock->recv_cb != NULL) { + isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL); + isc__nm_readcb(sock, req, ISC_R_TIMEDOUT, false); } - isc__nm_readcb(sock, req, result, async); - if (result == ISC_R_TIMEDOUT && - isc__nmsocket_streamdns_timer_running(sock)) - { - destroy = false; + + if (isc__nmsocket_timer_running(sock)) { + /* Timer was restarted, bail-out */ + return; } + + isc__nmsocket_clearcb(sock); + + goto destroy; } - if (destroy) { - isc__nmsocket_prep_destroy(sock); + isc_dnsstream_assembler_clear(sock->streamdns.input); + + /* Nobody expects the callback if isc_nm_read() wasn't called */ + if (!sock->client || sock->reading) { + sock->reading = false; + + if (sock->recv_cb != NULL) { + isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL); + isc__nmsocket_clearcb(sock); + isc__nm_readcb(sock, req, result, async); + } } + +destroy: + isc__nmsocket_prep_destroy(sock); } void @@ -533,6 +534,10 @@ streamdns_try_close_unused(isc_nmsocket_t *sock) { * The socket is unused after calling the callback. Let's close * the underlying connection. */ + /* FIXME: call failed_read_cb(?) */ + if (sock->outerhandle != NULL) { + isc_nmhandle_detach(&sock->outerhandle); + } isc__nmsocket_prep_destroy(sock); } } @@ -833,7 +838,7 @@ isc__nm_streamdns_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, sock->recv_cb = cb; sock->recv_cbarg = cbarg; - sock->recv_read = true; + sock->reading = true; isc_nmhandle_attach(handle, &sock->recv_handle); /* @@ -955,29 +960,6 @@ isc__nm_streamdns_stoplistening(isc_nmsocket_t *sock) { isc__nmsocket_stop(sock); } -static void -streamdns_cancelread_cb(void *arg) { - isc_nmsocket_t *sock = arg; - REQUIRE(VALID_NMSOCK(sock)); - - streamdns_failed_read_cb(sock, ISC_R_EOF, false); - isc__nmsocket_detach(&sock); -} - -void -isc__nm_streamdns_cancelread(isc_nmhandle_t *handle) { - isc_nmsocket_t *sock = NULL; - - REQUIRE(VALID_NMHANDLE(handle)); - REQUIRE(VALID_NMSOCK(handle->sock)); - REQUIRE(handle->sock->type == isc_nm_streamdnssocket); - - sock = handle->sock; - - isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); - isc_async_run(sock->worker->loop, streamdns_cancelread_cb, sock); -} - void isc__nmhandle_streamdns_cleartimeout(isc_nmhandle_t *handle) { isc_nmsocket_t *sock = NULL; diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 7b70fb8688c..11b05b639e5 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -154,16 +154,12 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) { REQUIRE(VALID_UVREQ(req)); REQUIRE(VALID_NMHANDLE(req->handle)); - if (sock->timedout) { + INSIST(sock->connecting); + + if (sock->timedout || status == UV_ETIMEDOUT) { + /* Connection timed-out */ result = ISC_R_TIMEDOUT; goto error; - } else if (!sock->connecting) { - /* - * The connect was cancelled from timeout; just clean up - * the req. - */ - isc__nm_uvreq_put(&req); - return; } else if (isc__nm_closing(worker)) { /* Network manager shutting down */ result = ISC_R_SHUTTINGDOWN; @@ -172,10 +168,6 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) { /* Connection canceled */ result = ISC_R_CANCELED; goto error; - } else if (status == UV_ETIMEDOUT) { - /* Timeout status code here indicates hard error */ - result = ISC_R_TIMEDOUT; - goto error; } else if (status == UV_EADDRINUSE) { /* * On FreeBSD the TCP connect() call sometimes results in a @@ -225,7 +217,8 @@ error: void isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, - isc_nm_cb_t cb, void *cbarg, unsigned int timeout) { + isc_nm_cb_t connect_cb, void *connect_cbarg, + unsigned int timeout) { isc_result_t result = ISC_R_SUCCESS; isc_nmsocket_t *sock = NULL; isc__nm_uvreq_t *req = NULL; @@ -238,7 +231,7 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, REQUIRE(peer != NULL); if (isc__nm_closing(worker)) { - cb(NULL, ISC_R_SHUTTINGDOWN, cbarg); + connect_cb(NULL, ISC_R_SHUTTINGDOWN, connect_cbarg); return; } @@ -246,7 +239,7 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, result = isc__nm_socket(sa_family, SOCK_STREAM, 0, &fd); if (result != ISC_R_SUCCESS) { - cb(NULL, result, cbarg); + connect_cb(NULL, result, connect_cbarg); return; } @@ -258,8 +251,8 @@ isc_nm_tcpconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, sock->client = true; req = isc__nm_uvreq_get(sock); - req->cb.connect = cb; - req->cbarg = cbarg; + req->cb.connect = connect_cb; + req->cbarg = connect_cbarg; req->peer = *peer; req->local = *local; req->handle = isc__nmhandle_get(sock, &req->peer, &sock->iface); @@ -632,6 +625,7 @@ isc__nm_tcp_stoplistening(isc_nmsocket_t *sock) { /* Stop the parent */ sock->closed = true; + isc__nmsocket_prep_destroy(sock); } @@ -662,18 +656,12 @@ isc__nm_tcp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result, isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - if (!sock->recv_read) { - goto destroy; - } - sock->recv_read = false; - if (sock->recv_cb != NULL) { isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL); isc__nmsocket_clearcb(sock); isc__nm_readcb(sock, req, result, async); } -destroy: isc__nmsocket_prep_destroy(sock); } @@ -694,7 +682,6 @@ isc__nm_tcp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) { sock->recv_cb = cb; sock->recv_cbarg = cbarg; - sock->recv_read = true; /* Initialize the timer */ if (sock->read_timeout == 0) { @@ -745,7 +732,6 @@ isc__nm_tcp_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_tid()); - REQUIRE(sock->reading); REQUIRE(buf != NULL); netmgr = sock->worker->netmgr; @@ -925,6 +911,10 @@ accept_connection(isc_nmsocket_t *csock) { */ isc_nmhandle_detach(&handle); + if (csock->statichandle != NULL) { + INSIST(csock->recv_cb != NULL); + } + /* * sock is now attached to the handle. */ @@ -1184,14 +1174,10 @@ tcp_close_connect_cb(uv_handle_t *handle) { void isc__nm_tcp_shutdown(isc_nmsocket_t *sock) { - isc__networker_t *worker = NULL; - REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_tid()); REQUIRE(sock->type == isc_nm_tcpsocket); - worker = sock->worker; - /* * If the socket is active, mark it inactive and * continue. If it isn't active, stop now. @@ -1201,9 +1187,7 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock) { } sock->active = false; - if (sock->accepting) { - return; - } + INSIST(!sock->accepting); if (sock->connecting) { isc_nmsocket_t *tsock = NULL; @@ -1212,12 +1196,9 @@ isc__nm_tcp_shutdown(isc_nmsocket_t *sock) { return; } - if (sock->statichandle != NULL) { - if (isc__nm_closing(worker)) { - isc__nm_failed_read_cb(sock, ISC_R_SHUTTINGDOWN, false); - } else { - isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false); - } + /* There's a handle attached to the socket (from accept or connect) */ + if (sock->statichandle) { + isc__nm_failed_read_cb(sock, ISC_R_SHUTTINGDOWN, false); return; } @@ -1243,7 +1224,6 @@ isc__nmhandle_tcp_set_manual_timer(isc_nmhandle_t *handle, const bool manual) { REQUIRE(sock->type == isc_nm_tcpsocket); REQUIRE(sock->tid == isc_tid()); REQUIRE(!sock->reading); - REQUIRE(!sock->recv_read); sock->manual_read_timer = manual; } diff --git a/lib/isc/netmgr/tlsstream.c b/lib/isc/netmgr/tlsstream.c index 6b85bed76aa..5de570a97b8 100644 --- a/lib/isc/netmgr/tlsstream.c +++ b/lib/isc/netmgr/tlsstream.c @@ -45,6 +45,42 @@ #define TLS_MAX_SEND_BUF_SIZE (UINT16_MAX + UINT16_MAX / 2) +#ifdef ISC_NETMGR_TRACE +ISC_ATTR_UNUSED static const char * +tls_status2str(int tls_status) { + switch (tls_status) { + case SSL_ERROR_NONE: + return ("SSL_ERROR_NONE"); + case SSL_ERROR_ZERO_RETURN: + return ("SSL_ERROR_ZERO_RETURN"); + case SSL_ERROR_WANT_WRITE: + return ("SSL_ERROR_WANT_WRITE"); + case SSL_ERROR_WANT_READ: + return ("SSL_ERROR_WANT_READ"); + case SSL_ERROR_SSL: + return ("SSL_ERROR_SSL"); + default: + UNREACHABLE(); + } +} + +ISC_ATTR_UNUSED static const char * +state2str(int state) { + switch (state) { + case TLS_INIT: + return ("TLS_INIT"); + case TLS_HANDSHAKE: + return ("TLS_HANDSHAKE"); + case TLS_IO: + return ("TLS_IO"); + case TLS_CLOSED: + return ("TLS_CLOSED"); + default: + UNREACHABLE(); + } +} +#endif /* ISC_NETMGR_TRACE */ + static isc_result_t tls_error_to_result(const int tls_err, const int tls_state, isc_tls_t *tls) { switch (tls_err) { @@ -62,6 +98,9 @@ tls_error_to_result(const int tls_err, const int tls_state, isc_tls_t *tls) { } } +static void +tls_read_start(isc_nmsocket_t *restrict sock); + static void tls_read_stop(isc_nmsocket_t *sock); @@ -76,9 +115,6 @@ static void tls_readcb(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region, void *cbarg); -static void -tls_close_direct(void *arg); - static void async_tls_do_bio(isc_nmsocket_t *sock); @@ -117,9 +153,7 @@ inactive(isc_nmsocket_t *sock) { static void tls_call_connect_cb(isc_nmsocket_t *sock, isc_nmhandle_t *handle, const isc_result_t result) { - if (sock->connect_cb == NULL) { - return; - } + INSIST(sock->connect_cb != NULL); sock->connect_cb(handle, result, sock->connect_cbarg); if (result != ISC_R_SUCCESS) { isc__nmsocket_clearcb(handle->sock); @@ -194,7 +228,12 @@ tls_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { } } - if (finish && eresult == ISC_R_SUCCESS && tlssock->reading) { + if (finish) { + /* + * If wrapping up, call tls_failed_read() - it will care of + * socket de-initialisation and calling the read callback, if + * necessary. + */ tls_failed_read_cb(tlssock, ISC_R_EOF); } else if (eresult == ISC_R_SUCCESS) { tls_do_bio(tlssock, NULL, NULL, false); @@ -218,12 +257,11 @@ tls_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { } static void -tls_failed_read_cb(isc_nmsocket_t *sock, const isc_result_t result) { - bool destroy = true; - +tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(result != ISC_R_SUCCESS); + /* This is TLS counterpart of isc__nm_failed_connect_cb() */ if (!sock->tlsstream.server && (sock->tlsstream.state == TLS_INIT || sock->tlsstream.state == TLS_HANDSHAKE) && @@ -235,37 +273,60 @@ tls_failed_read_cb(isc_nmsocket_t *sock, const isc_result_t result) { tls_call_connect_cb(sock, handle, result); isc__nmsocket_clearcb(sock); isc_nmhandle_detach(&handle); - } else if (sock->reading && sock->recv_cb != NULL && - sock->statichandle != NULL && - (sock->recv_read || result == ISC_R_TIMEDOUT)) - { - sock->recv_read = false; - sock->recv_cb(sock->statichandle, result, NULL, - sock->recv_cbarg); - if (result == ISC_R_TIMEDOUT && - (sock->outerhandle == NULL || - isc__nmsocket_timer_running(sock->outerhandle->sock))) - { - destroy = false; + goto destroy; + } + + isc__nmsocket_timer_stop(sock); + + /* Nobody is reading from the socket yet */ + if (sock->statichandle == NULL) { + goto destroy; + } + + /* This is TLS counterpart of isc__nmsocket_readtimeout_cb() */ + if (sock->client && result == ISC_R_TIMEDOUT) { + INSIST(sock->statichandle != NULL); + + if (sock->recv_cb != NULL) { + isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL); + isc__nm_readcb(sock, req, ISC_R_TIMEDOUT, false); + } + + if (isc__nmsocket_timer_running(sock)) { + /* Timer was restarted, bail-out */ + return; } + + isc__nmsocket_clearcb(sock); + + goto destroy; } - if (destroy) { - isc__nmsocket_prep_destroy(sock); + /* + * We don't need to check for .nsending, as the callbacks will be + * cleared at the time the tls_senddone() tries to call it for the + * second time. + */ + + if (sock->recv_cb != NULL) { + isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL); + isc__nmsocket_clearcb(sock); + isc__nm_readcb(sock, req, result, false); } + +destroy: + isc__nmsocket_prep_destroy(sock); } void isc__nm_tls_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result, - bool async) { - UNUSED(async); - + bool async ISC_ATTR_UNUSED) { if (!inactive(sock) && sock->tlsstream.state == TLS_IO) { tls_do_bio(sock, NULL, NULL, true); - } else if (sock->recv_read) { - tls_read_stop(sock); - tls_failed_read_cb(sock, result); + return; } + + tls_failed_read_cb(sock, result); } static void @@ -459,13 +520,10 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data, int rv = 0; size_t len = 0; int saved_errno = 0; - bool was_reading; REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_tid()); - was_reading = sock->reading; - /* * Clear the TLS error queue so that SSL_get_error() and SSL I/O * routine calls will not get affected by prior error statuses. @@ -592,7 +650,7 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data, /* Decrypt and pass data from network to client */ if (sock->tlsstream.state >= TLS_IO && sock->recv_cb != NULL && - was_reading && sock->statichandle != NULL && !finish) + sock->statichandle != NULL && sock->reading && !finish) { bool was_new_data = false; uint8_t recv_buf[TLS_BUF_SIZE]; @@ -693,7 +751,6 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data, pending = tls_process_outgoing(sock, finish, send_data); if (pending > 0 && tls_status != SSL_ERROR_SSL) { - /* We'll continue in tls_senddone */ return; } @@ -717,19 +774,20 @@ tls_do_bio(isc_nmsocket_t *sock, isc_region_t *received_data, { return; } else if (sock->reading == false && - sock->tlsstream.state > TLS_HANDSHAKE) + sock->tlsstream.state == TLS_HANDSHAKE) { - /* We need to read data when doing handshake even if - * 'sock->reading == false' */ + /* + * We need to read data when doing handshake even if + * 'sock->reading == false'. It will be stopped when + * handshake is completed. + */ + tls_read_start(sock); + return; + } else if (sock->reading == false) { return; } - INSIST(VALID_NMHANDLE(sock->outerhandle)); - - isc_nm_read(sock->outerhandle, tls_readcb, sock); - if (!sock->manual_read_timer) { - isc__nmsocket_timer_start(sock); - } + tls_read_start(sock); return; default: result = tls_error_to_result(tls_status, sock->tlsstream.state, @@ -766,6 +824,7 @@ tls_readcb(isc_nmhandle_t *handle, isc_result_t result, isc_region_t *region, return; } + REQUIRE(handle == tlssock->outerhandle); tls_do_bio(tlssock, region, NULL, false); } @@ -1022,16 +1081,29 @@ isc__nm_tls_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) { sock->recv_cb = cb; sock->recv_cbarg = cbarg; - sock->recv_read = true; sock->reading = true; async_tls_do_bio(sock); } static void -tls_read_stop(isc_nmsocket_t *sock) { - sock->reading = false; +tls_read_start(isc_nmsocket_t *restrict sock) { + if (sock->tlsstream.reading) { + return; + } + sock->tlsstream.reading = true; + + INSIST(VALID_NMHANDLE(sock->outerhandle)); + + isc_nm_read(sock->outerhandle, tls_readcb, sock); + if (!sock->manual_read_timer) { + isc__nmsocket_timer_start(sock); + } +} +static void +tls_read_stop(isc_nmsocket_t *sock) { + sock->tlsstream.reading = false; if (sock->outerhandle != NULL) { isc_nm_read_stop(sock->outerhandle); } @@ -1042,22 +1114,29 @@ isc__nm_tls_read_stop(isc_nmhandle_t *handle) { REQUIRE(VALID_NMHANDLE(handle)); REQUIRE(VALID_NMSOCK(handle->sock)); + handle->sock->reading = false; + tls_read_stop(handle->sock); } -static void -tls_close_direct(void *arg) { - isc_nmsocket_t *sock = arg; +void +isc__nm_tls_close(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); + REQUIRE(sock->type == isc_nm_tlssocket); + REQUIRE(!sock->closing); REQUIRE(sock->tid == isc_tid()); + REQUIRE(!sock->closed); + REQUIRE(!sock->closing); + + sock->closing = true; + /* * At this point we're certain that there are no * external references, we can close everything. */ + tls_read_stop(sock); if (sock->outerhandle != NULL) { - sock->reading = false; isc_nm_read_stop(sock->outerhandle); - isc_nmhandle_close(sock->outerhandle); isc_nmhandle_detach(&sock->outerhandle); } @@ -1072,22 +1151,6 @@ tls_close_direct(void *arg) { sock->tlsstream.state = TLS_CLOSED; } -void -isc__nm_tls_close(isc_nmsocket_t *sock) { - REQUIRE(VALID_NMSOCK(sock)); - REQUIRE(sock->type == isc_nm_tlssocket); - REQUIRE(!sock->closing); - - sock->closing = true; - - if (sock->tid == isc_tid()) { - /* no point in attempting to make the call asynchronous */ - tls_close_direct(sock); - } else { - isc_async_run(sock->worker->loop, tls_close_direct, sock); - } -} - void isc__nm_tls_stoplistening(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); @@ -1103,35 +1166,36 @@ tcp_connected(isc_nmhandle_t *handle, isc_result_t result, void *cbarg); void isc_nm_tlsconnect(isc_nm_t *mgr, isc_sockaddr_t *local, isc_sockaddr_t *peer, - isc_nm_cb_t cb, void *cbarg, isc_tlsctx_t *ctx, + isc_nm_cb_t connect_cb, void *connect_cbarg, + isc_tlsctx_t *ctx, isc_tlsctx_client_session_cache_t *client_sess_cache, unsigned int timeout) { - isc_nmsocket_t *nsock = NULL; + isc_nmsocket_t *sock = NULL; isc__networker_t *worker = &mgr->workers[isc_tid()]; REQUIRE(VALID_NM(mgr)); if (isc__nm_closing(worker)) { - cb(NULL, ISC_R_SHUTTINGDOWN, cbarg); + connect_cb(NULL, ISC_R_SHUTTINGDOWN, connect_cbarg); return; } - nsock = isc_mem_get(worker->mctx, sizeof(*nsock)); - isc__nmsocket_init(nsock, worker, isc_nm_tlssocket, local, NULL); - nsock->connect_cb = cb; - nsock->connect_cbarg = cbarg; - nsock->connect_timeout = timeout; - isc_tlsctx_attach(ctx, &nsock->tlsstream.ctx); - nsock->client = true; + sock = isc_mem_get(worker->mctx, sizeof(*sock)); + isc__nmsocket_init(sock, worker, isc_nm_tlssocket, local, NULL); + sock->connect_cb = connect_cb; + sock->connect_cbarg = connect_cbarg; + sock->connect_timeout = timeout; + isc_tlsctx_attach(ctx, &sock->tlsstream.ctx); + sock->client = true; if (client_sess_cache != NULL) { INSIST(isc_tlsctx_client_session_cache_getctx( client_sess_cache) == ctx); isc_tlsctx_client_session_cache_attach( - client_sess_cache, &nsock->tlsstream.client_sess_cache); + client_sess_cache, &sock->tlsstream.client_sess_cache); } - isc_nm_tcpconnect(mgr, local, peer, tcp_connected, nsock, - nsock->connect_timeout); + isc_nm_tcpconnect(mgr, local, peer, tcp_connected, sock, + sock->connect_timeout); } static void diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index 44de0fa5158..14558900f44 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -167,7 +167,6 @@ start_udp_child_job(void *arg) { goto done; } - sock->reading = true; done: result = isc_uverr2result(r); @@ -569,7 +568,7 @@ isc__nm_udp_read_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf, req->uvbuf.base = buf->base; req->uvbuf.len = nrecv; - sock->recv_read = false; + sock->reading = false; /* * The client isc_nm_read() expects just a single message, so we need to @@ -579,6 +578,7 @@ isc__nm_udp_read_cb(uv_udp_t *handle, ssize_t nrecv, const uv_buf_t *buf, if (sock->client) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); + isc__nmsocket_clearcb(sock); } REQUIRE(!sock->processing); @@ -844,44 +844,34 @@ isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result, REQUIRE(result != ISC_R_SUCCESS); REQUIRE(sock->tid == isc_tid()); + /* + * For UDP server socket, we don't have child socket via + * "accept", so we: + * - we continue to read + * - we don't clear the callbacks + * - we don't destroy it (only stoplistening could do that) + */ + if (sock->client) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); + } - /* Nobody expects the callback if isc_nm_read() wasn't called */ - if (!sock->recv_read) { - goto destroy; - } + /* Nobody expects the callback if isc_nm_read() wasn't called */ + if (sock->reading) { + sock->reading = false; if (sock->recv_cb != NULL) { isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL); - isc__nmsocket_clearcb(sock); isc__nm_readcb(sock, req, result, async); } - - sock->recv_read = false; - - destroy: - isc__nmsocket_prep_destroy(sock); - return; } - /* - * For UDP server socket, we don't have child socket via - * "accept", so we: - * - we continue to read - * - we don't clear the callbacks - * - we don't destroy it (only stoplistening could do that) - */ - if (!sock->recv_read) { + if (sock->client) { + isc__nmsocket_clearcb(sock); + isc__nmsocket_prep_destroy(sock); return; } - sock->recv_read = false; - - if (sock->recv_cb != NULL) { - isc__nm_uvreq_t *req = isc__nm_get_read_req(sock, NULL); - isc__nm_readcb(sock, req, result, async); - } } void @@ -896,7 +886,6 @@ isc__nm_udp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->type == isc_nm_udpsocket); REQUIRE(sock->statichandle == handle); - REQUIRE(!sock->recv_read); REQUIRE(sock->tid == isc_tid()); /* @@ -905,7 +894,7 @@ isc__nm_udp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) { */ sock->recv_cb = cb; sock->recv_cbarg = cbarg; - sock->recv_read = true; + sock->reading = true; if (isc__nm_closing(sock->worker)) { result = ISC_R_SHUTTINGDOWN; @@ -985,14 +974,10 @@ isc__nm_udp_close(isc_nmsocket_t *sock) { void isc__nm_udp_shutdown(isc_nmsocket_t *sock) { - isc__networker_t *worker = NULL; - REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_tid()); REQUIRE(sock->type == isc_nm_udpsocket); - worker = sock->worker; - /* * If the socket is active, mark it inactive and * continue. If it isn't active, stop now. @@ -1011,11 +996,7 @@ isc__nm_udp_shutdown(isc_nmsocket_t *sock) { * interested in the callback. */ if (sock->statichandle != NULL) { - if (isc__nm_closing(worker)) { - isc__nm_failed_read_cb(sock, ISC_R_SHUTTINGDOWN, false); - } else { - isc__nm_failed_read_cb(sock, ISC_R_CANCELED, false); - } + isc__nm_failed_read_cb(sock, ISC_R_SHUTTINGDOWN, false); return; } @@ -1030,28 +1011,3 @@ isc__nm_udp_shutdown(isc_nmsocket_t *sock) { isc__nmsocket_prep_destroy(sock->parent); } } - -static void -udp_cancelread_cb(void *arg) { - isc_nmsocket_t *sock = arg; - - REQUIRE(VALID_NMSOCK(sock)); - REQUIRE(sock->tid == isc_tid()); - REQUIRE(sock->client); - - isc__nm_failed_read_cb(sock, ISC_R_EOF, false); - isc__nmsocket_detach(&sock); -} - -void -isc__nm_udp_cancelread(isc_nmhandle_t *handle) { - REQUIRE(VALID_NMHANDLE(handle)); - - isc_nmsocket_t *sock = handle->sock; - - REQUIRE(VALID_NMSOCK(sock)); - REQUIRE(sock->type == isc_nm_udpsocket); - - isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); - isc_async_run(sock->worker->loop, udp_cancelread_cb, sock); -} diff --git a/lib/isccc/ccmsg.c b/lib/isccc/ccmsg.c index 1f0d2f00dd6..f52b0391527 100644 --- a/lib/isccc/ccmsg.c +++ b/lib/isccc/ccmsg.c @@ -46,42 +46,30 @@ static void recv_data(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, void *arg) { isccc_ccmsg_t *ccmsg = arg; - size_t size; - INSIST(VALID_CCMSG(ccmsg)); + REQUIRE(VALID_CCMSG(ccmsg)); - switch (eresult) { - case ISC_R_SHUTTINGDOWN: - case ISC_R_CANCELED: - case ISC_R_EOF: - ccmsg->result = eresult; - goto done; - case ISC_R_SUCCESS: - if (region == NULL) { - ccmsg->result = ISC_R_EOF; - goto done; - } - ccmsg->result = ISC_R_SUCCESS; - break; - default: - ccmsg->result = eresult; + REQUIRE(handle == ccmsg->handle); + if (eresult != ISC_R_SUCCESS) { goto done; } + REQUIRE(region != NULL); + if (!ccmsg->length_received) { if (region->length < sizeof(uint32_t)) { - ccmsg->result = ISC_R_UNEXPECTEDEND; + eresult = ISC_R_UNEXPECTEDEND; goto done; } ccmsg->size = ntohl(*(uint32_t *)region->base); if (ccmsg->size == 0) { - ccmsg->result = ISC_R_UNEXPECTEDEND; + eresult = ISC_R_UNEXPECTEDEND; goto done; } if (ccmsg->size > ccmsg->maxsize) { - ccmsg->result = ISC_R_RANGE; + eresult = ISC_R_RANGE; goto done; } @@ -100,13 +88,12 @@ recv_data(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, /* We have some data in the buffer, read it */ - size = ISC_MIN(isc_buffer_availablelength(ccmsg->buffer), - region->length); + size_t size = ISC_MIN(isc_buffer_availablelength(ccmsg->buffer), + region->length); isc_buffer_putmem(ccmsg->buffer, region->base, size); isc_region_consume(region, size); if (isc_buffer_usedlength(ccmsg->buffer) == ccmsg->size) { - ccmsg->result = ISC_R_SUCCESS; goto done; } @@ -115,7 +102,12 @@ recv_data(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, done: isc_nm_read_stop(handle); - ccmsg->cb(handle, ccmsg->result, ccmsg->cbarg); + if (ccmsg->reading) { + ccmsg->reading = false; + ccmsg->recv_cb(handle, eresult, ccmsg->recv_cbarg); + } + + return; } void @@ -129,9 +121,9 @@ isccc_ccmsg_init(isc_mem_t *mctx, isc_nmhandle_t *handle, .magic = CCMSG_MAGIC, .maxsize = 0xffffffffU, /* Largest message possible. */ .mctx = mctx, - .handle = handle, - .result = ISC_R_UNEXPECTED /* None yet. */ }; + + isc_nmhandle_attach(handle, &ccmsg->handle); } void @@ -149,23 +141,42 @@ isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg) { isc_buffer_free(&ccmsg->buffer); } - ccmsg->cb = cb; - ccmsg->cbarg = cbarg; - ccmsg->result = ISC_R_UNEXPECTED; /* unknown right now */ + ccmsg->recv_cb = cb; + ccmsg->recv_cbarg = cbarg; ccmsg->length_received = false; - isc_nm_read(ccmsg->handle, recv_data, ccmsg); ccmsg->reading = true; + isc_nm_read(ccmsg->handle, recv_data, ccmsg); } -void -isccc_ccmsg_cancelread(isccc_ccmsg_t *ccmsg) { +static void +ccmsg_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { + isccc_ccmsg_t *ccmsg = arg; + REQUIRE(VALID_CCMSG(ccmsg)); - if (ccmsg->reading) { - isc_nm_read_stop(ccmsg->handle); - ccmsg->reading = false; + INSIST(ccmsg->send_cb != NULL); + ccmsg->send_cb(handle, eresult, ccmsg->send_cbarg); + ccmsg->send_cb = NULL; + + if (eresult != ISC_R_SUCCESS && ccmsg->reading) { + recv_data(handle, eresult, NULL, ccmsg); } + + isc_nmhandle_detach(&handle); +} + +void +isccc_ccmsg_sendmessage(isccc_ccmsg_t *ccmsg, isc_region_t *region, + isc_nm_cb_t cb, void *cbarg) { + REQUIRE(VALID_CCMSG(ccmsg)); + REQUIRE(ccmsg->send_cb == NULL); + + ccmsg->send_cb = cb; + ccmsg->send_cbarg = cbarg; + + isc_nmhandle_ref(ccmsg->handle); + isc_nm_send(ccmsg->handle, region, ccmsg_senddone, ccmsg); } void @@ -177,4 +188,16 @@ isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg) { if (ccmsg->buffer != NULL) { isc_buffer_free(&ccmsg->buffer); } + if (ccmsg->handle != NULL) { + isc_nmhandle_close(ccmsg->handle); + isc_nmhandle_detach(&ccmsg->handle); + } +} + +void +isccc_ccmsg_toregion(isccc_ccmsg_t *ccmsg, isccc_region_t *ccregion) { + REQUIRE(VALID_CCMSG(ccmsg)); + + ccregion->rstart = isc_buffer_base(ccmsg->buffer); + ccregion->rend = isc_buffer_used(ccmsg->buffer); } diff --git a/lib/isccc/include/isccc/ccmsg.h b/lib/isccc/include/isccc/ccmsg.h index a648226524b..8404252a56f 100644 --- a/lib/isccc/include/isccc/ccmsg.h +++ b/lib/isccc/include/isccc/ccmsg.h @@ -38,6 +38,8 @@ #include #include +#include + /*% ISCCC Message Structure */ typedef struct isccc_ccmsg { /* private (don't touch!) */ @@ -48,11 +50,11 @@ typedef struct isccc_ccmsg { unsigned int maxsize; isc_mem_t *mctx; isc_nmhandle_t *handle; - isc_nm_cb_t cb; - void *cbarg; + isc_nm_cb_t recv_cb; + void *recv_cbarg; + isc_nm_cb_t send_cb; + void *send_cbarg; bool reading; - /* public (read-only) */ - isc_result_t result; } isccc_ccmsg_t; ISC_LANG_BEGINDECLS @@ -109,14 +111,12 @@ isccc_ccmsg_readmessage(isccc_ccmsg_t *ccmsg, isc_nm_cb_t cb, void *cbarg); */ void -isccc_ccmsg_cancelread(isccc_ccmsg_t *ccmsg); +isccc_ccmsg_sendmessage(isccc_ccmsg_t *ccmsg, isc_region_t *region, + isc_nm_cb_t cb, void *cbarg); /*% - * Cancel a readmessage() call. The event will still be posted with a - * CANCELED result code. - * - * Requires: + * Sends region over the command channel message. * - *\li "ccmsg" be valid. + * CAVEAT: Only a single send message can be scheduled at the time. */ void @@ -134,4 +134,7 @@ isccc_ccmsg_invalidate(isccc_ccmsg_t *ccmsg); * sockets, etc. */ +void +isccc_ccmsg_toregion(isccc_ccmsg_t *ccmsg, isccc_region_t *ccregion); + ISC_LANG_ENDDECLS diff --git a/tests/dns/dispatch_test.c b/tests/dns/dispatch_test.c index e58ccb6acd6..afd3e54b57d 100644 --- a/tests/dns/dispatch_test.c +++ b/tests/dns/dispatch_test.c @@ -322,13 +322,14 @@ server_senddone(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { static void nameserver(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, - void *cbarg) { + void *cbarg ISC_ATTR_UNUSED) { isc_region_t response1, response2; static unsigned char buf1[16]; static unsigned char buf2[16]; - UNUSED(eresult); - UNUSED(cbarg); + if (eresult != ISC_R_SUCCESS) { + return; + } memmove(buf1, region->base, 12); memset(buf1 + 12, 0, 4); diff --git a/tests/isc/Makefile.am b/tests/isc/Makefile.am index 4d72be6bece..a6c3227146e 100644 --- a/tests/isc/Makefile.am +++ b/tests/isc/Makefile.am @@ -113,6 +113,7 @@ tcp_test_SOURCES = \ tcp_test.c \ netmgr_common.h \ netmgr_common.c \ + stream_shutdown.c \ uv_wrap.h tcpdns_test_CPPFLAGS = \ @@ -141,6 +142,7 @@ tls_test_SOURCES = \ tls_test.c \ netmgr_common.h \ netmgr_common.c \ + stream_shutdown.c \ uv_wrap.h tlsdns_test_CPPFLAGS = \ diff --git a/tests/isc/netmgr_common.c b/tests/isc/netmgr_common.c index fa8faedbbf5..6ed42318b89 100644 --- a/tests/isc/netmgr_common.c +++ b/tests/isc/netmgr_common.c @@ -108,24 +108,13 @@ isc_nm_recv_cb_t connect_readcb = NULL; int setup_netmgr_test(void **state) { - char *env_workers = getenv("ISC_TASK_WORKERS"); - size_t nworkers; - tcp_connect_addr = (isc_sockaddr_t){ .length = 0 }; isc_sockaddr_fromin6(&tcp_connect_addr, &in6addr_loopback, 0); tcp_listen_addr = (isc_sockaddr_t){ .length = 0 }; isc_sockaddr_fromin6(&tcp_listen_addr, &in6addr_loopback, stream_port); - if (env_workers != NULL) { - workers = atoi(env_workers); - } else { - workers = isc_os_ncpus(); - } - INSIST(workers > 0); - nworkers = ISC_MAX(ISC_MIN(workers, 32), 1); - - esends = NSENDS * nworkers; + esends = NSENDS * workers; atomic_store(&nsends, esends); @@ -227,7 +216,7 @@ teardown_netmgr_test(void **state ISC_ATTR_UNUSED) { return (0); } -static void +void stop_listening(void *arg ISC_ATTR_UNUSED) { isc_nm_stoplistening(listen_sock); isc_nmsocket_close(&listen_sock); @@ -237,24 +226,23 @@ stop_listening(void *arg ISC_ATTR_UNUSED) { /* Callbacks */ void -noop_recv_cb(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, - void *cbarg) { - UNUSED(handle); - UNUSED(eresult); - UNUSED(region); - UNUSED(cbarg); +noop_recv_cb(isc_nmhandle_t *handle ISC_ATTR_UNUSED, + isc_result_t eresult ISC_ATTR_UNUSED, + isc_region_t *region ISC_ATTR_UNUSED, + void *cbarg ISC_ATTR_UNUSED) { + F(); } -unsigned int -noop_accept_cb(isc_nmhandle_t *handle, unsigned int result, void *cbarg) { - UNUSED(handle); - UNUSED(cbarg); +isc_result_t +noop_accept_cb(isc_nmhandle_t *handle ISC_ATTR_UNUSED, unsigned int eresult, + void *cbarg ISC_ATTR_UNUSED) { + F(); - if (result == ISC_R_SUCCESS) { + if (eresult == ISC_R_SUCCESS) { (void)atomic_fetch_add(&saccepts, 1); } - return (0); + return (ISC_R_SUCCESS); } void @@ -278,10 +266,8 @@ connect_send_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { case ISC_R_SHUTTINGDOWN: case ISC_R_CANCELED: case ISC_R_CONNECTIONRESET: - /* Send failed, we need to stop reading too */ - if (stream) { - isc_nm_read_stop(handle); - } else { + /* Abort */ + if (!stream) { isc_nm_cancelread(handle); } break; @@ -337,6 +323,10 @@ connect_read_cb(isc_nmhandle_t *handle, isc_result_t eresult, return; } + /* This will initiate one more read callback */ + if (stream) { + isc_nmhandle_close(handle); + } break; case ISC_R_TIMEDOUT: case ISC_R_EOF: @@ -352,10 +342,6 @@ connect_read_cb(isc_nmhandle_t *handle, isc_result_t eresult, } isc_refcount_decrement(&active_creads); - - if (stream) { - isc_nm_read_stop(handle); - } isc_nmhandle_detach(&handle); } @@ -399,8 +385,6 @@ listen_send_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { F(); - isc_refcount_decrement(&active_ssends); - switch (eresult) { case ISC_R_CANCELED: case ISC_R_CONNECTIONRESET: @@ -418,6 +402,7 @@ listen_send_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { assert_int_equal(eresult, ISC_R_SUCCESS); } + isc_refcount_decrement(&active_ssends); isc_nmhandle_detach(&sendhandle); } @@ -431,11 +416,6 @@ listen_read_cb(isc_nmhandle_t *handle, isc_result_t eresult, F(); switch (eresult) { - case ISC_R_CANCELED: - case ISC_R_CONNECTIONRESET: - case ISC_R_EOF: - case ISC_R_SHUTTINGDOWN: - break; case ISC_R_SUCCESS: memmove(&magic, region->base, sizeof(magic)); assert_true(magic == send_magic); @@ -449,18 +429,21 @@ listen_read_cb(isc_nmhandle_t *handle, isc_result_t eresult, memmove(&magic, region->base, sizeof(magic)); assert_true(magic == send_magic); - if (magic == send_magic) { - if (!noanswer) { - isc_nmhandle_t *sendhandle = NULL; - isc_nmhandle_attach(handle, &sendhandle); - isc_refcount_increment0(&active_ssends); - isc_nmhandle_setwritetimeout(sendhandle, - T_IDLE); - isc_nm_send(sendhandle, &send_msg, - listen_send_cb, cbarg); - } - return; + if (!noanswer) { + /* Answer and continue to listen */ + isc_nmhandle_t *sendhandle = NULL; + isc_nmhandle_attach(handle, &sendhandle); + isc_refcount_increment0(&active_ssends); + isc_nmhandle_setwritetimeout(sendhandle, T_IDLE); + isc_nm_send(sendhandle, &send_msg, listen_send_cb, + cbarg); } + /* Continue to listen */ + return; + case ISC_R_CANCELED: + case ISC_R_CONNECTIONRESET: + case ISC_R_EOF: + case ISC_R_SHUTTINGDOWN: break; default: fprintf(stderr, "%s(%p, %s, %p)\n", __func__, handle, @@ -469,7 +452,6 @@ listen_read_cb(isc_nmhandle_t *handle, isc_result_t eresult, } isc_refcount_decrement(&active_sreads); - isc_nmhandle_detach(&handle); } @@ -488,6 +470,9 @@ listen_accept_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { do_saccepts_shutdown(loopmgr); } + isc_nmhandle_attach(handle, &(isc_nmhandle_t *){ NULL }); + isc_refcount_increment0(&active_sreads); + return (eresult); } @@ -508,6 +493,7 @@ stream_accept_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { } isc_refcount_increment0(&active_sreads); + isc_nmhandle_attach(handle, &readhandle); isc_nm_read(handle, listen_read_cb, readhandle); @@ -670,11 +656,12 @@ noresponse_readcb(isc_nmhandle_t *handle, isc_result_t eresult, UNUSED(region); UNUSED(cbarg); + F(); + assert_true(eresult == ISC_R_CANCELED || eresult == ISC_R_CONNECTIONRESET || eresult == ISC_R_EOF); isc_refcount_decrement(&active_creads); - isc_nmhandle_detach(&handle); isc_loopmgr_shutdown(loopmgr); @@ -685,6 +672,8 @@ noresponse_sendcb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { UNUSED(cbarg); UNUSED(eresult); + F(); + assert_non_null(handle); atomic_fetch_add(&csends, 1); isc_nmhandle_detach(&handle); @@ -697,8 +686,6 @@ noresponse_connectcb(isc_nmhandle_t *handle, isc_result_t eresult, isc_nmhandle_t *readhandle = NULL; isc_nmhandle_t *sendhandle = NULL; - UNUSED(handle); - F(); isc_refcount_decrement(&active_cconnects); diff --git a/tests/isc/netmgr_common.h b/tests/isc/netmgr_common.h index 3ff3f62fef1..0e86282b720 100644 --- a/tests/isc/netmgr_common.h +++ b/tests/isc/netmgr_common.h @@ -201,15 +201,15 @@ extern isc_nm_recv_cb_t connect_readcb; fprintf(stderr, "%s:%s:%d:%s = %" PRId64 "\n", __func__, __FILE__, \ __LINE__, #v, atomic_load(&v)) #define P(v) fprintf(stderr, #v " = %" PRId64 "\n", v) -#define F() \ - fprintf(stderr, "%s(%p, %s, %p)\n", __func__, handle, \ +#define F() \ + fprintf(stderr, "%u:%s(%p, %s, %p)\n", isc_tid(), __func__, handle, \ isc_result_totext(eresult), cbarg) -#define isc_loopmgr_shutdown(loopmgr) \ - { \ - fprintf(stderr, "%s:%s:%d:isc_loopmgr_shutdown(%p)\n", \ - __func__, __FILE__, __LINE__, loopmgr); \ - isc_loopmgr_shutdown(loopmgr); \ +#define isc_loopmgr_shutdown(loopmgr) \ + { \ + fprintf(stderr, "%u:%s:%s:%d:isc_loopmgr_shutdown(%p)\n", \ + isc_tid(), __func__, __FILE__, __LINE__, loopmgr); \ + isc_loopmgr_shutdown(loopmgr); \ } #else #define X(v) @@ -234,8 +234,9 @@ void noop_recv_cb(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, void *cbarg); -unsigned int -noop_accept_cb(isc_nmhandle_t *handle, unsigned int result, void *cbarg); +isc_result_t +noop_accept_cb(isc_nmhandle_t *handle ISC_ATTR_UNUSED, unsigned int result, + void *cbarg ISC_ATTR_UNUSED); void connect_send_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg); @@ -325,3 +326,20 @@ int stream_recv_send_teardown(void **state ISC_ATTR_UNUSED); void stream_recv_send_connect(void *arg); + +int +stream_shutdownconnect_setup(void **state ISC_ATTR_UNUSED); +void +stream_shutdownconnect(void **state ISC_ATTR_UNUSED); +int +stream_shutdownconnect_teardown(void **state ISC_ATTR_UNUSED); + +int +stream_shutdownread_setup(void **state ISC_ATTR_UNUSED); +void +stream_shutdownread(void **state ISC_ATTR_UNUSED); +int +stream_shutdownread_teardown(void **state ISC_ATTR_UNUSED); + +void +stop_listening(void *arg ISC_ATTR_UNUSED); diff --git a/tests/isc/stream_shutdown.c b/tests/isc/stream_shutdown.c new file mode 100644 index 00000000000..71d6a99ff00 --- /dev/null +++ b/tests/isc/stream_shutdown.c @@ -0,0 +1,171 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +#include /* IWYU pragma: keep */ +#include +#include +#include +#include +#include + +/* + * As a workaround, include an OpenSSL header file before including cmocka.h, + * because OpenSSL 3.1.0 uses __attribute__(malloc), conflicting with a + * redefined malloc in cmocka.h. + */ +#include + +#define UNIT_TESTING +#include + +#include "netmgr_common.h" + +#include + +/* + * FIXME: This really needs two network managers, so there's predictable result + * when shuttingdown the netmgr - right now there's a race whether the listening + * or connecting sockets gets shutdown first + */ + +static void +shutdownconnect_connectcb(isc_nmhandle_t *handle, isc_result_t eresult, + void *cbarg) { + F(); + + assert_non_null(handle); + assert_int_equal(eresult, ISC_R_SHUTTINGDOWN); + assert_null(cbarg); + + isc_refcount_decrement(&active_cconnects); + + atomic_fetch_add(&cconnects, 1); +} + +int +stream_shutdownconnect_setup(void **state ISC_ATTR_UNUSED) { + int r = setup_netmgr_test(state); + return (r); +} + +void +stream_shutdownconnect(void **state ISC_ATTR_UNUSED) { + isc_result_t result = stream_listen(stream_accept_cb, NULL, 128, NULL, + &listen_sock); + assert_int_equal(result, ISC_R_SUCCESS); + isc_loop_teardown(mainloop, stop_listening, listen_sock); + + /* Schedule the shutdown before the connect */ + isc_loopmgr_shutdown(loopmgr); + + stream_connect(shutdownconnect_connectcb, NULL, T_CONNECT); +} + +int +stream_shutdownconnect_teardown(void **state ISC_ATTR_UNUSED) { + X(cconnects); + X(csends); + X(creads); + + atomic_assert_int_eq(cconnects, 1); + atomic_assert_int_eq(csends, 0); + atomic_assert_int_eq(creads, 0); + + return (teardown_netmgr_test(state)); +} + +/* Issue the shutdown before reading */ + +static void +shutdownread_readcb(isc_nmhandle_t *handle, isc_result_t eresult, + isc_region_t *region, void *cbarg) { + F(); + assert_non_null(handle); + assert_true(eresult == ISC_R_SHUTTINGDOWN || + eresult == ISC_R_CONNECTIONRESET || eresult == ISC_R_EOF); + assert_non_null(region); + assert_null(cbarg); + + atomic_fetch_add(&creads, 1); + isc_nmhandle_detach(&handle); + isc_refcount_decrement(&active_creads); +} + +static void +shutdownread_sendcb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { + F(); + assert_non_null(handle); + assert_true(eresult == ISC_R_SUCCESS || eresult == ISC_R_SHUTTINGDOWN || + eresult == ISC_R_CONNECTIONRESET || eresult == ISC_R_EOF); + assert_null(cbarg); + + atomic_fetch_add(&csends, 1); + + isc_nmhandle_detach(&handle); + isc_refcount_decrement(&active_csends); +} + +static void +shutdownread_connectcb(isc_nmhandle_t *handle, isc_result_t eresult, + void *cbarg) { + F(); + + assert_non_null(handle); + assert_int_equal(eresult, ISC_R_SUCCESS); + assert_null(cbarg); + + isc_refcount_decrement(&active_cconnects); + + atomic_fetch_add(&cconnects, 1); + + /* Schedule the shutdown before read and send */ + isc_loopmgr_shutdown(loopmgr); + + isc_refcount_increment0(&active_creads); + isc_nmhandle_ref(handle); + isc_nm_read(handle, shutdownread_readcb, cbarg); + + isc_refcount_increment0(&active_csends); + isc_nmhandle_ref(handle); + isc_nm_send(handle, (isc_region_t *)&send_msg, shutdownread_sendcb, + cbarg); +} + +int +stream_shutdownread_setup(void **state ISC_ATTR_UNUSED) { + int r = setup_netmgr_test(state); + return (r); +} + +void +stream_shutdownread(void **state ISC_ATTR_UNUSED) { + isc_result_t result = stream_listen(stream_accept_cb, NULL, 128, NULL, + &listen_sock); + assert_int_equal(result, ISC_R_SUCCESS); + isc_loop_teardown(mainloop, stop_listening, listen_sock); + + stream_connect(shutdownread_connectcb, NULL, T_CONNECT); +} + +int +stream_shutdownread_teardown(void **state ISC_ATTR_UNUSED) { + X(cconnects); + X(csends); + X(creads); + + atomic_assert_int_eq(cconnects, 1); + atomic_assert_int_eq(csends, 1); + atomic_assert_int_eq(creads, 1); + + return (teardown_netmgr_test(state)); +} diff --git a/tests/isc/tcp_test.c b/tests/isc/tcp_test.c index 4239e178992..5b990218c85 100644 --- a/tests/isc/tcp_test.c +++ b/tests/isc/tcp_test.c @@ -57,6 +57,16 @@ ISC_LOOP_TEST_IMPL(tcp_noresponse) { return; } +ISC_LOOP_TEST_IMPL(tcp_shutdownconnect) { + stream_shutdownconnect(arg); + return; +} + +ISC_LOOP_TEST_IMPL(tcp_shutdownread) { + stream_shutdownread(arg); + return; +} + ISC_LOOP_TEST_IMPL(tcp_timeout_recovery) { stream_timeout_recovery(arg); return; @@ -114,6 +124,10 @@ ISC_TEST_LIST_START ISC_TEST_ENTRY_CUSTOM(tcp_noop, stream_noop_setup, stream_noop_teardown) ISC_TEST_ENTRY_CUSTOM(tcp_noresponse, stream_noresponse_setup, stream_noresponse_teardown) +ISC_TEST_ENTRY_CUSTOM(tcp_shutdownconnect, stream_shutdownconnect_setup, + stream_shutdownconnect_teardown) +ISC_TEST_ENTRY_CUSTOM(tcp_shutdownread, stream_shutdownread_setup, + stream_shutdownread_teardown) ISC_TEST_ENTRY_CUSTOM(tcp_timeout_recovery, stream_timeout_recovery_setup, stream_timeout_recovery_teardown) ISC_TEST_ENTRY_CUSTOM(tcp_recv_one, stream_recv_one_setup, diff --git a/tests/isc/tcpdns_test.c b/tests/isc/tcpdns_test.c index 8005f537b2e..3fedd9e9d4a 100644 --- a/tests/isc/tcpdns_test.c +++ b/tests/isc/tcpdns_test.c @@ -47,13 +47,6 @@ /* TCPDNS */ -static void -stop_listening(void *arg ISC_ATTR_UNUSED) { - isc_nm_stoplistening(listen_sock); - isc_nmsocket_close(&listen_sock); - assert_null(listen_sock); -} - static void start_listening(uint32_t nworkers, isc_nm_accept_cb_t accept_cb, isc_nm_recv_cb_t recv_cb) { diff --git a/tests/isc/tls_test.c b/tests/isc/tls_test.c index e6c76aa07a0..58aaf131d2e 100644 --- a/tests/isc/tls_test.c +++ b/tests/isc/tls_test.c @@ -55,6 +55,16 @@ ISC_LOOP_TEST_IMPL(tls_noresponse) { return; } +ISC_LOOP_TEST_IMPL(tls_shutdownconnect) { + stream_shutdownconnect(arg); + return; +} + +ISC_LOOP_TEST_IMPL(tls_shutdownread) { + stream_shutdownread(arg); + return; +} + ISC_LOOP_TEST_IMPL(tls_timeout_recovery) { stream_timeout_recovery(arg); return; @@ -109,6 +119,10 @@ ISC_TEST_LIST_START ISC_TEST_ENTRY_CUSTOM(tls_noop, stream_noop_setup, stream_noop_teardown) ISC_TEST_ENTRY_CUSTOM(tls_noresponse, stream_noresponse_setup, stream_noresponse_teardown) +ISC_TEST_ENTRY_CUSTOM(tls_shutdownconnect, stream_shutdownconnect_setup, + stream_shutdownconnect_teardown) +ISC_TEST_ENTRY_CUSTOM(tls_shutdownread, stream_shutdownread_setup, + stream_shutdownread_teardown) ISC_TEST_ENTRY_CUSTOM(tls_timeout_recovery, stream_timeout_recovery_setup, stream_timeout_recovery_teardown) ISC_TEST_ENTRY_CUSTOM(tls_recv_one, stream_recv_one_setup, diff --git a/tests/isc/tlsdns_test.c b/tests/isc/tlsdns_test.c index 7c000687e20..b3a09b7726e 100644 --- a/tests/isc/tlsdns_test.c +++ b/tests/isc/tlsdns_test.c @@ -46,13 +46,6 @@ #include -static void -stop_listening(void *arg ISC_ATTR_UNUSED) { - isc_nm_stoplistening(listen_sock); - isc_nmsocket_close(&listen_sock); - assert_null(listen_sock); -} - static void start_listening(uint32_t nworkers, isc_nm_accept_cb_t accept_cb, isc_nm_recv_cb_t recv_cb) { diff --git a/tests/isc/udp_test.c b/tests/isc/udp_test.c index e88d7d4f2fb..c8482e2d1be 100644 --- a/tests/isc/udp_test.c +++ b/tests/isc/udp_test.c @@ -134,6 +134,15 @@ mock_recv_cb(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, UNUSED(cbarg); } +static void +udp_listen_read_cb(isc_nmhandle_t *handle, isc_result_t eresult, + isc_region_t *region, void *cbarg) { + if (eresult != ISC_R_SUCCESS) { + isc_refcount_increment0(&active_sreads); + } + listen_read_cb(handle, eresult, region, cbarg); +} + static void connect_nomemory_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { UNUSED(handle); @@ -145,13 +154,6 @@ connect_nomemory_cb(isc_nmhandle_t *handle, isc_result_t eresult, void *cbarg) { isc_loopmgr_shutdown(loopmgr); } -static void -stop_listening(void *arg ISC_ATTR_UNUSED) { - isc_nm_stoplistening(listen_sock); - isc_nmsocket_close(&listen_sock); - assert_null(listen_sock); -} - static void start_listening(uint32_t nworkers, isc_nm_recv_cb_t cb) { isc_result_t result = isc_nm_listenudp( @@ -616,6 +618,7 @@ udp_shutdown_read_connect_cb(isc_nmhandle_t *handle, isc_result_t eresult, isc_refcount_increment0(&active_creads); isc_nmhandle_attach(handle, &readhandle); isc_nm_read(handle, udp_shutdown_read_read_cb, cbarg); + assert_true(handle->sock->reading); /* Send */ isc_refcount_increment0(&active_csends); @@ -712,7 +715,7 @@ udp_cancel_read_read_cb(isc_nmhandle_t *handle, isc_result_t eresult, udp_cancel_read_send_cb, cbarg); } break; - case ISC_R_EOF: + case ISC_R_CANCELED: /* The read has been canceled */ atomic_fetch_add(&creads, 1); isc_loopmgr_shutdown(loopmgr); @@ -936,7 +939,7 @@ ISC_TEARDOWN_TEST_IMPL(udp_recv_one) { } ISC_LOOP_TEST_IMPL(udp_recv_one) { - start_listening(ISC_NM_LISTEN_ONE, listen_read_cb); + start_listening(ISC_NM_LISTEN_ONE, udp_listen_read_cb); udp__connect(NULL); } @@ -976,7 +979,7 @@ ISC_TEARDOWN_TEST_IMPL(udp_recv_two) { } ISC_LOOP_TEST_IMPL(udp_recv_two) { - start_listening(ISC_NM_LISTEN_ONE, listen_read_cb); + start_listening(ISC_NM_LISTEN_ONE, udp_listen_read_cb); udp__connect(NULL); udp__connect(NULL); @@ -1007,7 +1010,7 @@ ISC_TEARDOWN_TEST_IMPL(udp_recv_send) { } ISC_LOOP_TEST_IMPL(udp_recv_send) { - start_listening(ISC_NM_LISTEN_ALL, listen_read_cb); + start_listening(ISC_NM_LISTEN_ALL, udp_listen_read_cb); for (size_t i = 0; i < workers; i++) { isc_async_run(isc_loop_get(loopmgr, i), udp__connect, NULL); diff --git a/tests/libtest/isc.c b/tests/libtest/isc.c index 74e1c51ce76..777d2508eec 100644 --- a/tests/libtest/isc.c +++ b/tests/libtest/isc.c @@ -37,7 +37,7 @@ isc_log_t *lctx = NULL; isc_loop_t *mainloop = NULL; isc_loopmgr_t *loopmgr = NULL; isc_nm_t *netmgr = NULL; -unsigned int workers = -1; +unsigned int workers = 0; static void adjustnofile(void) { @@ -76,9 +76,14 @@ setup_loopmgr(void **state ISC_ATTR_UNUSED) { if (env_workers != NULL) { workers = atoi(env_workers); } - if (workers < 2 || workers > 1000) { + + if (workers == 0) { + workers = isc_os_ncpus(); + /* We always need at least two loops for some of the tests */ - workers = isc_os_ncpus() + 1; + if (workers < 2) { + workers = 2; + } } isc_loopmgr_create(mctx, workers, &loopmgr); diff --git a/tests/ns/netmgr_wrap.c b/tests/ns/netmgr_wrap.c index ef5b7dd4c18..725392fc5a8 100644 --- a/tests/ns/netmgr_wrap.c +++ b/tests/ns/netmgr_wrap.c @@ -22,9 +22,9 @@ #include #if ISC_NETMGR_TRACE -#define FLARG \ - , const char *file ISC_ATTR_UNUSED, unsigned int line ISC_ATTR_UNUSED, \ - const char *func ISC_ATTR_UNUSED +#define FLARG \ + , const char *func ISC_ATTR_UNUSED, const char *file ISC_ATTR_UNUSED, \ + unsigned int line ISC_ATTR_UNUSED #else #define FLARG #endif @@ -35,8 +35,13 @@ atomic_uint_fast32_t client_refs[32]; atomic_uintptr_t client_addrs[32]; +#if ISC_NETMGR_TRACE +void +isc_nmhandle__attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp FLARG) { +#else void -isc__nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp FLARG) { +isc_nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp) { +#endif ns_client_t *client = (ns_client_t *)source; int i; @@ -58,8 +63,13 @@ isc__nmhandle_attach(isc_nmhandle_t *source, isc_nmhandle_t **targetp FLARG) { return; } +#if ISC_NETMGR_TRACE +void +isc_nmhandle__detach(isc_nmhandle_t **handlep FLARG) { +#else void -isc__nmhandle_detach(isc_nmhandle_t **handlep FLARG) { +isc_nmhandle_detach(isc_nmhandle_t **handlep) { +#endif isc_nmhandle_t *handle = *handlep; ns_client_t *client = (ns_client_t *)handle; int i;