Change single write timer to per-send timers

Previously, there was a single per-socket write timer that would get
restarted for every new write.  This turned out to be insufficient
because the other side could keep reseting the timer, and never reading
back the responses.

Change the single write timer to per-send timer which would in turn
reset the TCP connection on the first send timeout.

(cherry picked from commit a761aa59e3d988b53e2f42f45bce53f2bea863ec)

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 4a7cb681869a789184ece31c28174d25c8649a5f..fe9fa831aa7e409d8d3dd01441ed427c96124d21 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -325,15 +325,15 @@ struct isc__nm_uvreq {
        int magic;
        isc_nmsocket_t *sock;
        isc_nmhandle_t *handle;
-       char tcplen[2];       /* The TCP DNS message length */
-       uv_buf_t uvbuf;       /* translated isc_region_t, to be
-                              * sent or received */
-       isc_sockaddr_t local; /* local address */
-       isc_sockaddr_t peer;  /* peer address */
-       isc__nm_cb_t cb;      /* callback */
-       void *cbarg;          /* callback argument */
-       uv_pipe_t ipc;        /* used for sending socket
-                              * uv_handles to other threads */
+       char tcplen[2];        /* The TCP DNS message length */
+       uv_buf_t uvbuf;        /* translated isc_region_t, to be
+                               * sent or received */
+       isc_sockaddr_t local;  /* local address */
+       isc_sockaddr_t peer;   /* peer address */
+       isc__nm_cb_t cb;       /* callback */
+       void *cbarg;           /* callback argument */
+       isc_nm_timer_t *timer; /* TCP write timer */
+
        union {
                uv_handle_t handle;
                uv_req_t req;
@@ -777,9 +777,7 @@ struct isc_nmsocket {
        /*%
         * 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;
@@ -1592,7 +1590,7 @@ 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);
+isc__nmsocket_writetimeout_cb(void *data, isc_result_t eresult);
 
 /*%<
  *

diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index b7e553974f2c46a90b20333745f989b8db08ced0..41e4bce616316f9454b541b4fcdffbf36474e9c3 100644 (file)
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -1943,11 +1943,15 @@ isc__nm_accept_connection_log(isc_result_t result, bool can_log_quota) {
 }
 
 void
-isc__nmsocket_writetimeout_cb(uv_timer_t *timer) {
-       isc_nmsocket_t *sock = uv_handle_get_data((uv_handle_t *)timer);
+isc__nmsocket_writetimeout_cb(void *data, isc_result_t eresult) {
+       isc__nm_uvreq_t *req = data;
+       isc_nmsocket_t *sock = NULL;
 
-       int r = uv_timer_stop(&sock->write_timer);
-       UV_RUNTIME_CHECK(uv_timer_stop, r);
+       REQUIRE(eresult == ISC_R_TIMEDOUT);
+       REQUIRE(VALID_UVREQ(req));
+       REQUIRE(VALID_NMSOCK(req->sock));
+
+       sock = req->sock;
 
        isc__nmsocket_reset(sock);
 }
@@ -2677,6 +2681,13 @@ isc__nm_async_detach(isc__networker_t *worker, isc__netievent_t *ev0) {
        nmhandle_detach_cb(&ievent->handle FLARG_PASS);
 }
 
+static void
+reset_shutdown(uv_handle_t *handle) {
+       isc_nmsocket_t *sock = uv_handle_get_data(handle);
+
+       isc__nmsocket_shutdown(sock);
+}
+
 void
 isc__nmsocket_reset(isc_nmsocket_t *sock) {
        REQUIRE(VALID_NMSOCK(sock));
@@ -2700,10 +2711,10 @@ isc__nmsocket_reset(isc_nmsocket_t *sock) {
                 * The real shutdown will be handled in the respective
                 * close functions.
                 */
-               int r = uv_tcp_close_reset(&sock->uv_handle.tcp, NULL);
+               int r = uv_tcp_close_reset(&sock->uv_handle.tcp,
+                                          reset_shutdown);
                UV_RUNTIME_CHECK(uv_tcp_close_reset, r);
        }
-       isc__nmsocket_shutdown(sock);
 }
 
 void
@@ -3238,7 +3249,8 @@ isc_nm_timer_detach(isc_nm_timer_t **timerp) {
        REQUIRE(VALID_NMSOCK(handle->sock));
 
        if (isc_refcount_decrement(&timer->references) == 1) {
-               uv_timer_stop(&timer->timer);
+               int r = uv_timer_stop(&timer->timer);
+               UV_RUNTIME_CHECK(uv_timer_stop, r);
                uv_close((uv_handle_t *)&timer->timer, timer_destroy);
        }
 }

diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 64914c29e384595b172481d6be098d2e84e7388e..6e602666c998b063bd63b8acc87a7be275532120 100644 (file)
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -78,9 +78,6 @@ quota_accept_cb(isc_quota_t *quota, void *sock0);
 static void
 failed_accept_cb(isc_nmsocket_t *sock, isc_result_t eresult);
 
-static void
-failed_send_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
-              isc_result_t eresult);
 static void
 stop_tcp_parent(isc_nmsocket_t *sock);
 static void
@@ -144,10 +141,6 @@ tcp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
        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) {
                isc__nm_closesocket(sock->fd);
@@ -537,10 +530,6 @@ isc__nm_async_tcplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
        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);
-
        LOCK(&sock->parent->lock);
 
        r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd);
@@ -713,19 +702,6 @@ destroy:
        }
 }
 
-static void
-failed_send_cb(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
-              isc_result_t eresult) {
-       REQUIRE(VALID_NMSOCK(sock));
-       REQUIRE(VALID_UVREQ(req));
-
-       if (req->cb.send != NULL) {
-               isc__nm_sendcb(sock, req, eresult, true);
-       } else {
-               isc__nm_uvreq_put(&req, sock);
-       }
-}
-
 void
 isc__nm_tcp_read(isc_nmhandle_t *handle, isc_nm_recv_cb_t cb, void *cbarg) {
        REQUIRE(VALID_NMHANDLE(handle));
@@ -990,10 +966,6 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
        UV_RUNTIME_CHECK(uv_timer_init, r);
        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) {
                result = isc__nm_uverr2result(r);
@@ -1104,20 +1076,20 @@ 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;
+       isc_nmsocket_t *sock = NULL;
 
        REQUIRE(VALID_UVREQ(uvreq));
-       REQUIRE(VALID_NMHANDLE(uvreq->handle));
+       REQUIRE(VALID_NMSOCK(uvreq->sock));
 
-       isc_nmsocket_t *sock = uvreq->sock;
+       sock = uvreq->sock;
 
-       if (--sock->writes == 0) {
-               int r = uv_timer_stop(&sock->write_timer);
-               UV_RUNTIME_CHECK(uv_timer_stop, r);
-       }
+       isc_nm_timer_stop(uvreq->timer);
+       isc_nm_timer_detach(&uvreq->timer);
 
        if (status < 0) {
                isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]);
-               failed_send_cb(sock, uvreq, isc__nm_uverr2result(status));
+               isc__nm_failed_send_cb(sock, uvreq,
+                                      isc__nm_uverr2result(status));
                return;
        }
 
@@ -1141,7 +1113,7 @@ isc__nm_async_tcpsend(isc__networker_t *worker, isc__netievent_t *ev0) {
        result = tcp_send_direct(sock, uvreq);
        if (result != ISC_R_SUCCESS) {
                isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]);
-               failed_send_cb(sock, uvreq, result);
+               isc__nm_failed_send_cb(sock, uvreq, result);
        }
 }
 
@@ -1158,17 +1130,18 @@ 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) {
                return (isc__nm_uverr2result(r));
        }
 
+       isc_nm_timer_create(req->handle, isc__nmsocket_writetimeout_cb, req,
+                           &req->timer);
+       if (sock->write_timeout > 0) {
+               isc_nm_timer_start(req->timer, sock->write_timeout);
+       }
+
        return (ISC_R_SUCCESS);
 }
 
@@ -1239,17 +1212,6 @@ read_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);
@@ -1302,8 +1264,6 @@ 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));
@@ -1325,10 +1285,8 @@ tcp_close_direct(isc_nmsocket_t *sock) {
        isc__nmsocket_timer_stop(sock);
        isc__nm_stop_reading(sock);
 
-       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);
+       uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock);
+       uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb);
 }
 
 void

diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index 8fa2a43c5f384d0c6c00bb217e97a972dd94f0fa..e588527852dda035a1a064377cef561cf2aa55b0 100644 (file)
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -102,10 +102,6 @@ tcpdns_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
        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_CANCELED;
                goto error;
@@ -500,10 +496,6 @@ isc__nm_async_tcpdnslisten(isc__networker_t *worker, isc__netievent_t *ev0) {
        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);
-
        LOCK(&sock->parent->lock);
 
        r = uv_tcp_open(&sock->uv_handle.tcp, sock->fd);
@@ -958,10 +950,6 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
        UV_RUNTIME_CHECK(uv_timer_init, r);
        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) {
                result = isc__nm_uverr2result(r);
@@ -1096,14 +1084,12 @@ tcpdns_send_cb(uv_write_t *req, int status) {
        isc_nmsocket_t *sock = NULL;
 
        REQUIRE(VALID_UVREQ(uvreq));
-       REQUIRE(VALID_NMHANDLE(uvreq->handle));
+       REQUIRE(VALID_NMSOCK(uvreq->sock));
 
        sock = uvreq->sock;
 
-       if (--sock->writes == 0) {
-               int r = uv_timer_stop(&sock->write_timer);
-               UV_RUNTIME_CHECK(uv_timer_stop, r);
-       }
+       isc_nm_timer_stop(uvreq->timer);
+       isc_nm_timer_detach(&uvreq->timer);
 
        if (status < 0) {
                isc__nm_incstats(sock->mgr, sock->statsindex[STATID_SENDFAIL]);
@@ -1169,11 +1155,6 @@ 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) {
@@ -1181,6 +1162,12 @@ isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) {
                goto fail;
        }
 
+       isc_nm_timer_create(uvreq->handle, isc__nmsocket_writetimeout_cb, uvreq,
+                           &uvreq->timer);
+       if (sock->write_timeout > 0) {
+               isc_nm_timer_start(uvreq->timer, sock->write_timeout);
+       }
+
        return;
 fail:
        if (result != ISC_R_SUCCESS) {
@@ -1260,17 +1247,6 @@ read_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);
@@ -1323,7 +1299,6 @@ 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));
@@ -1339,10 +1314,8 @@ tcpdns_close_direct(isc_nmsocket_t *sock) {
        isc__nmsocket_timer_stop(sock);
        isc__nm_stop_reading(sock);
 
-       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);
+       uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock);
+       uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb);
 }
 
 void

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 77de1cdafdf55e03dae705e6c0fc77b6e4a517c1..ad3695581266653d0095c3c5924a285211764d3a 100644 (file)
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -50,9 +50,6 @@ udp_close_cb(uv_handle_t *handle);
 static void
 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);
 
@@ -235,10 +232,6 @@ isc__nm_async_udplisten(isc__networker_t *worker, isc__netievent_t *ev0) {
        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);
-
        LOCK(&sock->parent->lock);
 
        r = uv_udp_open(&sock->uv_handle.udp, sock->fd);
@@ -661,10 +654,6 @@ udp_connect_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) {
        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_udp_open(&sock->uv_handle.udp, sock->fd);
        if (r != 0) {
                isc__nm_incstats(sock->mgr, sock->statsindex[STATID_OPENFAIL]);
@@ -1020,17 +1009,6 @@ read_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);
@@ -1083,14 +1061,11 @@ 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());
 
-       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);
+       uv_handle_set_data((uv_handle_t *)&sock->read_timer, sock);
+       uv_close((uv_handle_t *)&sock->read_timer, read_timer_close_cb);
 }
 
 void