diff --git a/CHANGES b/CHANGES index 7597fce9d8..6e09a1491e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5807. [bug] Add a TCP "write" timer, and time out writing + connections after the "tcp-idle-timeout" period + has elapsed. [GL #3132] + 5806. [bug] An error in checking the "blackhole" ACL could cause DNS requests sent by named to fail if the destination address or prefix was specifically diff --git a/bin/tests/system/timeouts/setup.sh b/bin/tests/system/timeouts/setup.sh index 2e8fd6a6ba..c4019d2a27 100644 --- a/bin/tests/system/timeouts/setup.sh +++ b/bin/tests/system/timeouts/setup.sh @@ -20,6 +20,11 @@ copy_setports ns1/named.conf.in ns1/named.conf # tcp-initial-timeout interval # $PYTHON -c " +print('large IN TXT', end=' ') +for a in range(128): + print('\"%s\"' % ('A' * 240), end=' ') +print('') + for a in range(150000): print('%s IN NS a' % (a)) print('%s IN NS b' % (a))" > ns1/large.db diff --git a/bin/tests/system/timeouts/tests-tcp.py b/bin/tests/system/timeouts/tests-tcp.py index 2c5e99cc1c..e1f19608c8 100644 --- a/bin/tests/system/timeouts/tests-tcp.py +++ b/bin/tests/system/timeouts/tests-tcp.py @@ -51,7 +51,7 @@ def test_initial_timeout(port): try: (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) (response, rtime) = dns.query.receive_tcp(sock, timeout()) - except ConnectionResetError as e: + except ConnectionError as e: raise EOFError from e @@ -83,7 +83,7 @@ def test_idle_timeout(port): try: (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) (response, rtime) = dns.query.receive_tcp(sock, timeout()) - except ConnectionResetError as e: + except ConnectionError as e: raise EOFError from e @@ -152,7 +152,7 @@ def test_pipelining_timeout(port): try: (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) (response, rtime) = dns.query.receive_tcp(sock, timeout()) - except ConnectionResetError as e: + except ConnectionError as e: raise EOFError from e @@ -190,3 +190,33 @@ def test_long_axfr(port): if soa is not None: break assert soa is not None + + +@pytest.mark.dnspython +@pytest.mark.dnspython2 +def test_send_timeout(port): + import dns.query + + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.connect(("10.53.0.1", port)) + + # Send and receive single large RDATA over TCP + msg = create_msg("large.example.", "TXT") + (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) + (response, rtime) = dns.query.receive_tcp(sock, timeout()) + + # Send and receive 28 large (~32k) DNS queries that should + # fill the default maximum 208k TCP send buffer + for n in range(28): + (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) + + # configure idle interval is 5 seconds, sleep 6 to make sure we are + # above the interval + time.sleep(6) + + with pytest.raises(EOFError): + try: + (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) + (response, rtime) = dns.query.receive_tcp(sock, timeout()) + except ConnectionError as e: + raise EOFError from e diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index c9ae25023b..18e67bd992 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -70,3 +70,9 @@ Bug Fixes to ``none``. ``blackhole`` worked correctly when it was left unset, or if only positive-match elements were included. This has now been fixed. :gl:`#3157` + +- TCP connections could hang indefinitely if the TCP write buffers + were full because of the other party not reading sent data. This has + been fixed by adding a "write" timer. Connections that are hung + while writing will now time out after the ``tcp-idle-timeout`` period + has elapsed. :gl:`#3132` diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 6c52f638c8..671880137d 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -697,3 +697,6 @@ isc__nm_force_tid(int tid); * Force the thread ID to 'tid'. This is STRICTLY for use in unit * tests and should not be used in any production code. */ + +void +isc_nmhandle_setwritetimeout(isc_nmhandle_t *handle, uint64_t write_timeout); diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 2ce53402b2..188d90340e 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -950,10 +950,17 @@ struct isc_nmsocket { /*% * TCP read/connect timeout timers. */ - uv_timer_t timer; + uv_timer_t read_timer; uint64_t read_timeout; uint64_t connect_timeout; + /*% + * TCP write timeout timer. + */ + uv_timer_t write_timer; + uint64_t write_timeout; + int64_t writes; + /*% outer socket is for 'wrapped' sockets - e.g. tcpdns in tcp */ isc_nmsocket_t *outer; @@ -2071,12 +2078,25 @@ isc__nm_failed_connect_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req, void isc__nm_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result, bool async); -void -isc__nmsocket_connecttimeout_cb(uv_timer_t *timer); - void isc__nm_accept_connection_log(isc_result_t result, bool can_log_quota); +/* + * Timeout callbacks + */ +void +isc__nmsocket_connecttimeout_cb(uv_timer_t *timer); +void +isc__nmsocket_readtimeout_cb(uv_timer_t *timer); +void +isc__nmsocket_writetimeout_cb(uv_timer_t *timer); + +/*%< + * + * Maximum number of simultaneous handles in flight supported for a single + * connected TCPDNS socket. This value was chosen arbitrarily, and may be + * changed in the future. + */ #define STREAM_CLIENTS_PER_CONN 23 #define UV_RUNTIME_CHECK(func, ret) \ diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index ecdb26340d..ddcfe23276 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -604,6 +604,14 @@ isc_nm_maxudp(isc_nm_t *mgr, uint32_t maxudp) { atomic_store(&mgr->maxudp, maxudp); } +void +isc_nmhandle_setwritetimeout(isc_nmhandle_t *handle, uint64_t write_timeout) { + REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(VALID_NMSOCK(handle->sock)); + + handle->sock->write_timeout = write_timeout; +} + void isc_nm_settimeouts(isc_nm_t *mgr, uint32_t init, uint32_t idle, uint32_t keepalive, uint32_t advertised) { @@ -1977,7 +1985,7 @@ isc__nm_failed_connect_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req, REQUIRE(req->cb.connect != NULL); isc__nmsocket_timer_stop(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); INSIST(atomic_compare_exchange_strong(&sock->connecting, &(bool){ true }, false)); @@ -2070,7 +2078,21 @@ isc__nm_accept_connection_log(isc_result_t result, bool can_log_quota) { isc_result_totext(result)); } -static void +void +isc__nmsocket_writetimeout_cb(uv_timer_t *timer) { + isc_nmsocket_t *sock = uv_handle_get_data((uv_handle_t *)timer); + + int r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + + /* The shutdown will be handled in the respective close functions */ + r = uv_tcp_close_reset(&sock->uv_handle.tcp, NULL); + UV_RUNTIME_CHECK(uv_tcp_close_reset, r); + + isc__nmsocket_shutdown(sock); +} + +void isc__nmsocket_readtimeout_cb(uv_timer_t *timer) { isc_nmsocket_t *sock = uv_handle_get_data((uv_handle_t *)timer); @@ -2108,7 +2130,7 @@ isc__nmsocket_timer_restart(isc_nmsocket_t *sock) { return; } - r = uv_timer_start(&sock->timer, + r = uv_timer_start(&sock->read_timer, isc__nmsocket_connecttimeout_cb, sock->connect_timeout + 10, 0); UV_RUNTIME_CHECK(uv_timer_start, r); @@ -2120,7 +2142,8 @@ isc__nmsocket_timer_restart(isc_nmsocket_t *sock) { return; } - r = uv_timer_start(&sock->timer, isc__nmsocket_readtimeout_cb, + r = uv_timer_start(&sock->read_timer, + isc__nmsocket_readtimeout_cb, sock->read_timeout, 0); UV_RUNTIME_CHECK(uv_timer_start, r); } @@ -2130,7 +2153,7 @@ bool isc__nmsocket_timer_running(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); - return (uv_is_active((uv_handle_t *)&sock->timer)); + return (uv_is_active((uv_handle_t *)&sock->read_timer)); } void @@ -2152,7 +2175,7 @@ isc__nmsocket_timer_stop(isc_nmsocket_t *sock) { /* uv_timer_stop() is idempotent, no need to check if running */ - r = uv_timer_stop(&sock->timer); + r = uv_timer_stop(&sock->read_timer); UV_RUNTIME_CHECK(uv_timer_stop, r); } @@ -2404,7 +2427,7 @@ isc_nmhandle_cleartimeout(isc_nmhandle_t *handle) { default: handle->sock->read_timeout = 0; - if (uv_is_active((uv_handle_t *)&handle->sock->timer)) { + if (uv_is_active((uv_handle_t *)&handle->sock->read_timer)) { isc__nmsocket_timer_stop(handle->sock); } } @@ -2446,6 +2469,8 @@ isc_nmhandle_keepalive(isc_nmhandle_t *handle, bool value) { atomic_store(&sock->keepalive, value); sock->read_timeout = value ? atomic_load(&sock->mgr->keepalive) : atomic_load(&sock->mgr->idle); + sock->write_timeout = value ? atomic_load(&sock->mgr->keepalive) + : atomic_load(&sock->mgr->idle); break; #if HAVE_LIBNGHTTP2 case isc_nm_tlssocket: diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 45339244c7..e179bc05b0 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -140,8 +140,13 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&sock->uv_handle.handle, sock); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd); if (r != 0) { @@ -170,7 +175,8 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { } isc__nm_incstats(sock, STATID_CONNECT); - uv_handle_set_data((uv_handle_t *)&sock->timer, &req->uv_req.connect); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, + &req->uv_req.connect); isc__nmsocket_timer_start(sock); atomic_store(&sock->connected, true); @@ -231,7 +237,7 @@ tcp_connect_cb(uv_connect_t *uvreq, int status) { REQUIRE(sock->tid == isc_nm_tid()); isc__nmsocket_timer_stop(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); if (!atomic_load(&sock->connecting)) { return; @@ -527,10 +533,13 @@ isc__nm_async_tcplisten(isc__networker_t *worker, isc__netievent_t *ev0) { /* This keeps the socket alive after everything else is gone */ isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); LOCK(&sock->parent->lock); @@ -974,9 +983,13 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&csock->uv_handle.handle, csock); - r = uv_timer_init(&worker->loop, &csock->timer); + r = uv_timer_init(&worker->loop, &csock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&csock->timer, csock); + uv_handle_set_data((uv_handle_t *)&csock->read_timer, csock); + + r = uv_timer_init(&worker->loop, &csock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&csock->write_timer, csock); r = uv_accept(&ssock->uv_handle.stream, &csock->uv_handle.stream); if (r != 0) { @@ -1071,6 +1084,13 @@ isc__nm_tcp_send(isc_nmhandle_t *handle, const isc_region_t *region, uvreq->cb.send = cb; uvreq->cbarg = cbarg; + if (sock->write_timeout == 0) { + sock->write_timeout = + (atomic_load(&sock->keepalive) + ? atomic_load(&sock->mgr->keepalive) + : atomic_load(&sock->mgr->idle)); + } + ievent = isc__nm_get_netievent_tcpsend(sock->mgr, sock, uvreq); isc__nm_maybe_enqueue_ievent(&sock->mgr->workers[sock->tid], (isc__netievent_t *)ievent); @@ -1081,11 +1101,17 @@ isc__nm_tcp_send(isc_nmhandle_t *handle, const isc_region_t *region, static void tcp_send_cb(uv_write_t *req, int status) { isc__nm_uvreq_t *uvreq = (isc__nm_uvreq_t *)req->data; + REQUIRE(VALID_UVREQ(uvreq)); REQUIRE(VALID_NMHANDLE(uvreq->handle)); isc_nmsocket_t *sock = uvreq->sock; + if (--sock->writes == 0) { + int r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + } + if (status < 0) { isc__nm_incstats(sock, STATID_SENDFAIL); failed_send_cb(sock, uvreq, isc__nm_uverr2result(status)); @@ -1129,6 +1155,11 @@ tcp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { return (ISC_R_CANCELED); } + r = uv_timer_start(&sock->write_timer, isc__nmsocket_writetimeout_cb, + sock->write_timeout, 0); + UV_RUNTIME_CHECK(uv_timer_start, r); + RUNTIME_CHECK(sock->writes++ >= 0); + r = uv_write(&req->uv_req.write, &sock->uv_handle.stream, &req->uvbuf, 1, tcp_send_cb); if (r < 0) { @@ -1192,7 +1223,7 @@ tcp_close_cb(uv_handle_t *handle) { } static void -timer_close_cb(uv_handle_t *handle) { +read_timer_close_cb(uv_handle_t *handle) { isc_nmsocket_t *sock = uv_handle_get_data(handle); uv_handle_set_data(handle, NULL); @@ -1205,6 +1236,17 @@ timer_close_cb(uv_handle_t *handle) { } } +static void +write_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)); + + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); +} + static void stop_tcp_child(isc_nmsocket_t *sock) { REQUIRE(sock->type == isc_nm_tcpsocket); @@ -1257,6 +1299,8 @@ stop_tcp_parent(isc_nmsocket_t *sock) { static void tcp_close_direct(isc_nmsocket_t *sock) { + int r; + REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_nm_tid()); REQUIRE(atomic_load(&sock->closing)); @@ -1278,8 +1322,10 @@ tcp_close_direct(isc_nmsocket_t *sock) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); - uv_close((uv_handle_t *)&sock->timer, timer_close_cb); + r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + uv_close((uv_handle_t *)&sock->write_timer, write_timer_close_cb); } void @@ -1401,7 +1447,7 @@ isc__nm_async_tcpcancel(isc__networker_t *worker, isc__netievent_t *ev0) { REQUIRE(sock->tid == isc_nm_tid()); UNUSED(worker); - uv_timer_stop(&sock->timer); + uv_timer_stop(&sock->read_timer); isc__nm_tcp_failed_read_cb(sock, ISC_R_EOF); } diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c index ce31e30c86..9244320ec0 100644 --- a/lib/isc/netmgr/tcpdns.c +++ b/lib/isc/netmgr/tcpdns.c @@ -37,13 +37,6 @@ #include "netmgr-int.h" #include "uv-compat.h" -/*%< - * - * Maximum number of simultaneous handles in flight supported for a single - * connected TCPDNS socket. This value was chosen arbitrarily, and may be - * changed in the future. - */ - static atomic_uint_fast32_t last_tcpdnsquota_log = ATOMIC_VAR_INIT(0); static bool @@ -105,8 +98,13 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&sock->uv_handle.handle, sock); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; @@ -144,7 +142,8 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { } isc__nm_incstats(sock, STATID_CONNECT); - uv_handle_set_data((uv_handle_t *)&sock->timer, &req->uv_req.connect); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, + &req->uv_req.connect); isc__nmsocket_timer_start(sock); atomic_store(&sock->connected, true); @@ -205,7 +204,7 @@ tcpdns_connect_cb(uv_connect_t *uvreq, int status) { REQUIRE(sock->tid == isc_nm_tid()); isc__nmsocket_timer_stop(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); if (!atomic_load(&sock->connecting)) { return; @@ -497,9 +496,13 @@ isc__nm_async_tcpdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) { /* This keeps the socket alive after everything else is gone */ isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); LOCK(&sock->parent->lock); @@ -966,9 +969,13 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&csock->uv_handle.handle, csock); - r = uv_timer_init(&worker->loop, &csock->timer); + r = uv_timer_init(&worker->loop, &csock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&csock->timer, csock); + uv_handle_set_data((uv_handle_t *)&csock->read_timer, csock); + + r = uv_timer_init(&worker->loop, &csock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&csock->write_timer, csock); r = uv_accept(&ssock->uv_handle.stream, &csock->uv_handle.stream); if (r != 0) { @@ -1086,6 +1093,13 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region, uvreq->cb.send = cb; uvreq->cbarg = cbarg; + if (sock->write_timeout == 0) { + sock->write_timeout = + (atomic_load(&sock->keepalive) + ? atomic_load(&sock->mgr->keepalive) + : atomic_load(&sock->mgr->idle)); + } + ievent = isc__nm_get_netievent_tcpdnssend(sock->mgr, sock, uvreq); isc__nm_maybe_enqueue_ievent(&sock->mgr->workers[sock->tid], (isc__netievent_t *)ievent); @@ -1103,6 +1117,11 @@ tcpdns_send_cb(uv_write_t *req, int status) { sock = uvreq->sock; + if (--sock->writes == 0) { + int r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + } + if (status < 0) { isc__nm_incstats(sock, STATID_SENDFAIL); isc__nm_failed_send_cb(sock, uvreq, @@ -1169,6 +1188,11 @@ isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) { goto fail; } + r = uv_timer_start(&sock->write_timer, isc__nmsocket_writetimeout_cb, + sock->write_timeout, 0); + UV_RUNTIME_CHECK(uv_timer_start, r); + RUNTIME_CHECK(sock->writes++ >= 0); + r = uv_write(&uvreq->uv_req.write, &sock->uv_handle.stream, bufs, nbufs, tcpdns_send_cb); if (r < 0) { @@ -1238,7 +1262,7 @@ tcpdns_close_cb(uv_handle_t *handle) { } static void -timer_close_cb(uv_handle_t *timer) { +read_timer_close_cb(uv_handle_t *timer) { isc_nmsocket_t *sock = uv_handle_get_data(timer); uv_handle_set_data(timer, NULL); @@ -1253,6 +1277,17 @@ timer_close_cb(uv_handle_t *timer) { } } +static void +write_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)); + + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); +} + static void stop_tcpdns_child(isc_nmsocket_t *sock) { REQUIRE(sock->type == isc_nm_tcpdnssocket); @@ -1305,6 +1340,7 @@ stop_tcpdns_parent(isc_nmsocket_t *sock) { static void tcpdns_close_direct(isc_nmsocket_t *sock) { + int r; REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_nm_tid()); REQUIRE(atomic_load(&sock->closing)); @@ -1320,8 +1356,10 @@ tcpdns_close_direct(isc_nmsocket_t *sock) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); - uv_close((uv_handle_t *)&sock->timer, timer_close_cb); + r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + uv_close((uv_handle_t *)&sock->write_timer, write_timer_close_cb); } void diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index bd564bac25..d91e04489d 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -38,13 +38,6 @@ #include "openssl_shim.h" #include "uv-compat.h" -/*%< - * - * Maximum number of simultaneous handles in flight supported for a single - * connected TLSDNS socket. This value was chosen arbitrarily, and may be - * changed in the future. - */ - static atomic_uint_fast32_t last_tlsdnsquota_log = ATOMIC_VAR_INIT(0); static void @@ -121,9 +114,13 @@ tlsdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&sock->uv_handle.handle, sock); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; @@ -161,7 +158,8 @@ tlsdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { } isc__nm_incstats(sock, STATID_CONNECT); - uv_handle_set_data((uv_handle_t *)&sock->timer, &req->uv_req.connect); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, + &req->uv_req.connect); isc__nmsocket_timer_start(sock); atomic_store(&sock->connected, true); @@ -569,9 +567,13 @@ isc__nm_async_tlsdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) { /* This keeps the socket alive after everything else is gone */ isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); LOCK(&sock->parent->lock); @@ -1107,7 +1109,8 @@ tls_cycle_input(isc_nmsocket_t *sock) { sock->tls.pending_req = NULL; isc__nmsocket_timer_stop(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, + sock); INSIST(atomic_compare_exchange_strong( &sock->connecting, &(bool){ true }, false)); @@ -1163,6 +1166,11 @@ tls_write_cb(uv_write_t *req, int status) { isc__nm_uvreq_t *uvreq = (isc__nm_uvreq_t *)req->data; isc_nmsocket_t *sock = uvreq->sock; + if (--sock->writes == 0) { + int r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + } + free_senddata(sock); isc__nm_uvreq_put(&uvreq, sock); @@ -1236,6 +1244,12 @@ tls_cycle_output(isc_nmsocket_t *sock) { break; } + r = uv_timer_start(&sock->write_timer, + isc__nmsocket_writetimeout_cb, + sock->write_timeout, 0); + UV_RUNTIME_CHECK(uv_timer_start, r); + RUNTIME_CHECK(sock->writes++ >= 0); + r = uv_write(&req->uv_req.write, &sock->uv_handle.stream, &req->uvbuf, 1, tls_write_cb); if (r < 0) { @@ -1476,9 +1490,13 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) { UV_RUNTIME_CHECK(uv_tcp_init, r); uv_handle_set_data(&csock->uv_handle.handle, csock); - r = uv_timer_init(&worker->loop, &csock->timer); + r = uv_timer_init(&worker->loop, &csock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&csock->timer, csock); + uv_handle_set_data((uv_handle_t *)&csock->read_timer, csock); + + r = uv_timer_init(&worker->loop, &csock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&csock->write_timer, csock); r = uv_accept(&ssock->uv_handle.stream, &csock->uv_handle.stream); if (r != 0) { @@ -1627,6 +1645,13 @@ isc__nm_tlsdns_send(isc_nmhandle_t *handle, isc_region_t *region, uvreq->cb.send = cb; uvreq->cbarg = cbarg; + if (sock->write_timeout == 0) { + sock->write_timeout = + (atomic_load(&sock->keepalive) + ? atomic_load(&sock->mgr->keepalive) + : atomic_load(&sock->mgr->idle)); + } + ievent = isc__nm_get_netievent_tlsdnssend(sock->mgr, sock, uvreq); isc__nm_enqueue_ievent(&sock->mgr->workers[sock->tid], (isc__netievent_t *)ievent); @@ -1804,7 +1829,7 @@ tlsdns_close_cb(uv_handle_t *handle) { } static void -timer_close_cb(uv_handle_t *handle) { +read_timer_close_cb(uv_handle_t *handle) { isc_nmsocket_t *sock = uv_handle_get_data(handle); uv_handle_set_data(handle, NULL); @@ -1819,6 +1844,17 @@ timer_close_cb(uv_handle_t *handle) { } } +static void +write_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)); + + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); +} + static void stop_tlsdns_child(isc_nmsocket_t *sock) { REQUIRE(sock->type == isc_nm_tlsdnssocket); @@ -1872,6 +1908,8 @@ stop_tlsdns_parent(isc_nmsocket_t *sock) { static void tlsdns_close_direct(isc_nmsocket_t *sock) { + int r; + REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_nm_tid()); REQUIRE(atomic_load(&sock->closing)); @@ -1889,8 +1927,10 @@ tlsdns_close_direct(isc_nmsocket_t *sock) { isc__nmsocket_timer_stop(sock); isc__nm_stop_reading(sock); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); - uv_close((uv_handle_t *)&sock->timer, timer_close_cb); + r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + uv_close((uv_handle_t *)&sock->write_timer, write_timer_close_cb); } void diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c index 3573836785..e8ef4100f1 100644 --- a/lib/isc/netmgr/udp.c +++ b/lib/isc/netmgr/udp.c @@ -74,7 +74,10 @@ static void udp_close_cb(uv_handle_t *handle); static void -timer_close_cb(uv_handle_t *handle); +read_timer_close_cb(uv_handle_t *handle); + +static void +write_timer_close_cb(uv_handle_t *handle); static void udp_close_direct(isc_nmsocket_t *sock); @@ -265,9 +268,13 @@ route_connect_direct(isc_nmsocket_t *sock) { UV_RUNTIME_CHECK(uv_udp_init, r); uv_handle_set_data(&sock->uv_handle.handle, sock); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; @@ -441,9 +448,13 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) { /* This keeps the socket alive after everything else is gone */ isc__nmsocket_attach(sock, &(isc_nmsocket_t *){ NULL }); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); LOCK(&sock->parent->lock); @@ -857,9 +868,13 @@ udp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { UV_RUNTIME_CHECK(uv_udp_init, r); uv_handle_set_data(&sock->uv_handle.handle, sock); - r = uv_timer_init(&worker->loop, &sock->timer); + r = uv_timer_init(&worker->loop, &sock->read_timer); UV_RUNTIME_CHECK(uv_timer_init, r); - uv_handle_set_data((uv_handle_t *)&sock->timer, sock); + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + + r = uv_timer_init(&worker->loop, &sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); if (isc__nm_closing(sock)) { result = ISC_R_SHUTTINGDOWN; @@ -1210,7 +1225,7 @@ udp_close_cb(uv_handle_t *handle) { } static void -timer_close_cb(uv_handle_t *handle) { +read_timer_close_cb(uv_handle_t *handle) { isc_nmsocket_t *sock = uv_handle_get_data(handle); uv_handle_set_data(handle, NULL); @@ -1221,6 +1236,17 @@ timer_close_cb(uv_handle_t *handle) { } } +static void +write_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)); + + uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock); + uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb); +} + static void stop_udp_child(isc_nmsocket_t *sock) { REQUIRE(sock->type == isc_nm_udpsocket); @@ -1273,10 +1299,14 @@ stop_udp_parent(isc_nmsocket_t *sock) { static void udp_close_direct(isc_nmsocket_t *sock) { + int r; REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->tid == isc_nm_tid()); - uv_close((uv_handle_t *)&sock->timer, timer_close_cb); + r = uv_timer_stop(&sock->write_timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); + uv_handle_set_data((uv_handle_t *)&sock->write_timer, sock); + uv_close((uv_handle_t *)&sock->write_timer, write_timer_close_cb); } void diff --git a/lib/isc/netmgr/uv-compat.c b/lib/isc/netmgr/uv-compat.c index 104ba5118e..b7c0f7b0e2 100644 --- a/lib/isc/netmgr/uv-compat.c +++ b/lib/isc/netmgr/uv-compat.c @@ -42,6 +42,24 @@ isc_uv_udp_connect(uv_udp_t *handle, const struct sockaddr *addr) { } #endif /* UV_VERSION_HEX < UV_VERSION(1, 27, 0) */ +#if UV_VERSION_HEX < UV_VERSION(1, 32, 0) +int +uv_tcp_close_reset(uv_tcp_t *handle, uv_close_cb close_cb) { + if (setsockopt(handle->io_watcher.fd, SOL_SOCKET, SO_LINGER, + &(struct linger){ 1, 0 }, sizeof(struct linger)) == -1) + { +#if UV_VERSION_HEX >= UV_VERSION(1, 10, 0) + return (uv_translate_sys_error(errno)); +#else + return (-errno); +#endif /* UV_VERSION_HEX >= UV_VERSION(1, 10, 0) */ + } + + uv_close((uv_handle_t *)handle, close_cb); + return (0); +} +#endif /* UV_VERSION_HEX < UV_VERSION(1, 32, 0) */ + int isc_uv_udp_freebind(uv_udp_t *handle, const struct sockaddr *addr, unsigned int flags) { diff --git a/lib/isc/netmgr/uv-compat.h b/lib/isc/netmgr/uv-compat.h index 6797484a8a..387d3c6940 100644 --- a/lib/isc/netmgr/uv-compat.h +++ b/lib/isc/netmgr/uv-compat.h @@ -23,6 +23,10 @@ #define UV_VERSION(major, minor, patch) ((major << 16) | (minor << 8) | (patch)) +#if !defined(UV__ERR) +#define UV__ERR(x) (-(x)) +#endif + #if UV_VERSION_HEX < UV_VERSION(1, 19, 0) static inline void * uv_handle_get_data(const uv_handle_t *handle) { @@ -45,6 +49,11 @@ uv_req_set_data(uv_req_t *req, void *data) { } #endif /* UV_VERSION_HEX < UV_VERSION(1, 19, 0) */ +#if UV_VERSION_HEX < UV_VERSION(1, 32, 0) +int +uv_tcp_close_reset(uv_tcp_t *handle, uv_close_cb close_cb); +#endif + #if UV_VERSION_HEX < UV_VERSION(1, 34, 0) #define uv_sleep(msec) usleep(msec * 1000) #endif /* UV_VERSION_HEX < UV_VERSION(1, 34, 0) */ diff --git a/lib/isc/tests/netmgr_test.c b/lib/isc/tests/netmgr_test.c index 824668d4ba..b27c095b7d 100644 --- a/lib/isc/tests/netmgr_test.c +++ b/lib/isc/tests/netmgr_test.c @@ -437,6 +437,7 @@ connect_send(isc_nmhandle_t *handle) { isc_nmhandle_t *sendhandle = NULL; isc_refcount_increment0(&active_csends); isc_nmhandle_attach(handle, &sendhandle); + isc_nmhandle_setwritetimeout(handle, T_IDLE); if (atomic_fetch_sub(&nsends, 1) > 1) { isc_nm_send(sendhandle, (isc_region_t *)&send_msg, connect_send_cb, NULL); @@ -548,6 +549,7 @@ listen_read_cb(isc_nmhandle_t *handle, isc_result_t eresult, 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, (isc_region_t *)&send_msg, listen_send_cb, cbarg); }