diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 82953cb0d9..1fd4bc3e6e 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -62,6 +62,8 @@ static isc_result_t tcp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req); static void tcp_connect_cb(uv_connect_t *uvreq, int status); +static void +tcp_stop_cb(uv_handle_t *handle); static void tcp_connection_cb(uv_stream_t *server, int status); @@ -683,7 +685,20 @@ isc__nm_async_tcpstop(isc__networker_t *worker, isc__netievent_t *ev0) { RUNTIME_CHECK(atomic_compare_exchange_strong(&sock->closing, &(bool){ false }, true)); - tcp_close_direct(sock); + /* + * The order of the close operation is important here, the uv_close() + * gets scheduled in the reverse order, so we need to close the timer + * last, so its gone by the time we destroy the socket + */ + + /* 2. close the listening socket */ + isc__nmsocket_clearcb(sock); + isc__nm_stop_reading(sock); + uv_close(&sock->uv_handle.handle, tcp_stop_cb); + + /* 1. close the read timer */ + isc__nmsocket_timer_stop(sock); + uv_close(&sock->read_timer, NULL); (void)atomic_fetch_sub(&sock->parent->rchildren, 1); @@ -1217,25 +1232,12 @@ tcp_close_cb(uv_handle_t *handle) { tcp_close_sock(sock); } -static void -read_timer_close_cb(uv_handle_t *handle) { - isc_nmsocket_t *sock = uv_handle_get_data(handle); - uv_handle_set_data(handle, NULL); - - if (sock->parent) { - uv_close(&sock->uv_handle.handle, tcp_stop_cb); - } else if (uv_is_closing(&sock->uv_handle.handle)) { - tcp_close_sock(sock); - } else { - uv_close(&sock->uv_handle.handle, tcp_close_cb); - } -} - static void tcp_close_direct(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_tid()); REQUIRE(atomic_load(&sock->closing)); + REQUIRE(sock->parent == NULL); if (sock->server != NULL) { REQUIRE(VALID_NMSOCK(sock->server)); @@ -1251,12 +1253,31 @@ tcp_close_direct(isc_nmsocket_t *sock) { isc_quota_detach(&sock->quota); } - isc__nmsocket_clearcb(sock); - isc__nmsocket_timer_stop(sock); - isc__nm_stop_reading(sock); + /* + * The order of the close operation is important here, the uv_close() + * gets scheduled in the reverse order, so we need to close the timer + * last, so its gone by the time we destroy the socket + */ - uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); - uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); + if (!uv_is_closing(&sock->uv_handle.handle)) { + /* Normal order of operation */ + + /* 2. close the socket + destroy the socket in callback */ + isc__nmsocket_clearcb(sock); + isc__nm_stop_reading(sock); + uv_close(&sock->uv_handle.handle, tcp_close_cb); + + /* 1. close the timer */ + isc__nmsocket_timer_stop(sock); + uv_close((uv_handle_t *)&sock->read_timer, NULL); + } else { + /* The socket was already closed elsewhere */ + + /* 1. close the timer + destroy the socket in callback */ + isc__nmsocket_timer_stop(sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, tcp_close_cb); + } } void diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index 172d5b0740..47695e1528 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -63,6 +63,9 @@ tcpdns_connect_cb(uv_connect_t *uvreq, int status); static void tcpdns_connection_cb(uv_stream_t *server, int status); +static void +tcpdns_stop_cb(uv_handle_t *handle); + static void tcpdns_close_cb(uv_handle_t *uvhandle); @@ -646,7 +649,20 @@ isc__nm_async_tcpdnsstop(isc__networker_t *worker, isc__netievent_t *ev0) { RUNTIME_CHECK(atomic_compare_exchange_strong(&sock->closing, &(bool){ false }, true)); - tcpdns_close_direct(sock); + /* + * The order of the close operation is important here, the uv_close() + * gets scheduled in the reverse order, so we need to close the timer + * last, so its gone by the time we destroy the socket + */ + + /* 2. close the listening socket */ + isc__nmsocket_clearcb(sock); + isc__nm_stop_reading(sock); + uv_close(&sock->uv_handle.handle, tcpdns_stop_cb); + + /* 1. close the read timer */ + isc__nmsocket_timer_stop(sock); + uv_close(&sock->read_timer, NULL); (void)atomic_fetch_sub(&sock->parent->rchildren, 1); @@ -1270,22 +1286,6 @@ tcpdns_close_cb(uv_handle_t *handle) { tcpdns_close_sock(sock); } -static void -read_timer_close_cb(uv_handle_t *timer) { - isc_nmsocket_t *sock = uv_handle_get_data(timer); - uv_handle_set_data(timer, NULL); - - REQUIRE(VALID_NMSOCK(sock)); - - if (sock->parent) { - uv_close(&sock->uv_handle.handle, tcpdns_stop_cb); - } else if (uv_is_closing(&sock->uv_handle.handle)) { - tcpdns_close_sock(sock); - } else { - uv_close(&sock->uv_handle.handle, tcpdns_close_cb); - } -} - static void tcpdns_close_direct(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); @@ -1300,12 +1300,30 @@ tcpdns_close_direct(isc_nmsocket_t *sock) { isc_nmhandle_detach(&sock->recv_handle); } - isc__nmsocket_clearcb(sock); - isc__nmsocket_timer_stop(sock); - isc__nm_stop_reading(sock); + /* + * The order of the close operation is important here, the uv_close() + * gets scheduled in the reverse order, so we need to close the timer + * last, so its gone by the time we destroy the socket + */ - uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); - uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); + if (!uv_is_closing(&sock->uv_handle.handle)) { + /* Normal order of operation */ + + /* 2. close the socket + destroy the socket in callback */ + isc__nmsocket_clearcb(sock); + isc__nm_stop_reading(sock); + uv_close(&sock->uv_handle.handle, tcpdns_close_cb); + + /* 1. close the timer */ + uv_close((uv_handle_t *)&sock->read_timer, NULL); + } else { + /* The socket was already closed elsewhere */ + + /* 1. close the timer + destroy the socket in callback */ + isc__nmsocket_timer_stop(sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, tcpdns_close_cb); + } } void diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index 3ad86a3979..73826ca9c5 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -56,6 +56,9 @@ tlsdns_connect_cb(uv_connect_t *uvreq, int status); static void tlsdns_connection_cb(uv_stream_t *server, int status); +static void +tlsdns_stop_cb(uv_handle_t *handle); + static void tlsdns_close_cb(uv_handle_t *uvhandle); @@ -769,7 +772,20 @@ isc__nm_async_tlsdnsstop(isc__networker_t *worker, isc__netievent_t *ev0) { RUNTIME_CHECK(atomic_compare_exchange_strong(&sock->closing, &(bool){ false }, true)); - tlsdns_close_direct(sock); + /* + * The order of the close operation is important here, the uv_close() + * gets scheduled in the reverse order, so we need to close the timer + * last, so its gone by the time we destroy the socket + */ + + /* 2. close the listening socket */ + isc__nmsocket_clearcb(sock); + isc__nm_stop_reading(sock); + uv_close(&sock->uv_handle.handle, tlsdns_stop_cb); + + /* 1. close the read timer */ + isc__nmsocket_timer_stop(sock); + uv_close(&sock->read_timer, NULL); (void)atomic_fetch_sub(&sock->parent->rchildren, 1); @@ -1908,27 +1924,12 @@ tlsdns_close_cb(uv_handle_t *handle) { tlsdns_close_sock(sock); } -static void -read_timer_close_cb(uv_handle_t *handle) { - isc_nmsocket_t *sock = uv_handle_get_data(handle); - uv_handle_set_data(handle, NULL); - - REQUIRE(VALID_NMSOCK(sock)); - - if (sock->parent) { - uv_close(&sock->uv_handle.handle, tlsdns_stop_cb); - } else if (uv_is_closing(&sock->uv_handle.handle)) { - tlsdns_close_sock(sock); - } else { - uv_close(&sock->uv_handle.handle, tlsdns_close_cb); - } -} - static void tlsdns_close_direct(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_tid()); REQUIRE(atomic_load(&sock->closing)); + REQUIRE(sock->parent == NULL); REQUIRE(sock->tls.pending_req == NULL); @@ -1940,12 +1941,31 @@ tlsdns_close_direct(isc_nmsocket_t *sock) { isc_nmhandle_detach(&sock->recv_handle); } - isc__nmsocket_clearcb(sock); - isc__nmsocket_timer_stop(sock); - isc__nm_stop_reading(sock); + /* + * The order of the close operation is important here, the uv_close() + * gets scheduled in the reverse order, so we need to close the timer + * last, so its gone by the time we destroy the socket + */ - uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); - uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); + if (!uv_is_closing(&sock->uv_handle.handle)) { + /* Normal order of operation */ + + /* 2. close the socket + destroy the socket in callback */ + isc__nmsocket_clearcb(sock); + isc__nm_stop_reading(sock); + uv_close(&sock->uv_handle.handle, tlsdns_close_cb); + + /* 1. close the timer */ + isc__nmsocket_timer_stop(sock); + uv_close((uv_handle_t *)&sock->read_timer, NULL); + } else { + /* The socket was already closed elsewhere */ + + /* 1. close the timer + destroy the socket in callback */ + isc__nmsocket_timer_stop(sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, tlsdns_close_cb); + } } void diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index ff7cc4163d..58f31edc55 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -65,9 +65,6 @@ udp_send_cb(uv_udp_send_t *req, int status); static void udp_close_cb(uv_handle_t *handle); -static void -read_timer_close_cb(uv_handle_t *handle); - static uv_os_sock_t isc__nm_udp_lb_socket(isc_nm_t *mgr, sa_family_t sa_family) { isc_result_t result; @@ -1008,14 +1005,6 @@ udp_close_cb(uv_handle_t *handle) { } } -static void -read_timer_close_cb(uv_handle_t *handle) { - isc_nmsocket_t *sock = uv_handle_get_data(handle); - uv_handle_set_data(handle, NULL); - - uv_close(&sock->uv_handle.handle, udp_close_cb); -} - void isc__nm_udp_close(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); @@ -1031,7 +1020,20 @@ isc__nm_udp_close(isc_nmsocket_t *sock) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); + /* + * The order of the close operation is important here, the uv_close() + * gets scheduled in the reverse order, so we need to close the timer + * last, so its gone by the time we destroy the socket + */ + + /* 2. close the listening socket */ + isc__nmsocket_clearcb(sock); + isc__nm_stop_reading(sock); + uv_close(&sock->uv_handle.handle, udp_close_cb); + + /* 1. close the read timer */ + isc__nmsocket_timer_stop(sock); + uv_close((uv_handle_t *)&sock->read_timer, NULL); } void