clean up some handle/client reference counting errors in error cases.

We weren't consistent about who should unreference the handle in
case of network error. Make it consistent so that it's always the
client code responsibility to unreference the handle - either
in the callback or right away if send function failed and the callback
will never be called.

diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index a15cb07b9645d99abf78537c64f41d180346a255..36f63c8189dff1b3b35406b91b0bb3f083745f29 100644 (file)
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -470,8 +470,7 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
        REQUIRE(sock->type == isc_nm_tcpdnssocket);
 
        if (sock->outer == NULL) {
-               /* The socket is closed, just issue the callback */
-               cb(handle, ISC_R_FAILURE, cbarg);
+               /* The socket is closed */
                return (ISC_R_NOTCONNECTED);
        }
 

diff --git a/lib/isc/netmgr/udp.c b/lib/isc/netmgr/udp.c
index 936fa963d9c82d5af1418589a8ac81f0769e4b69..cb14425703cc3e64bf58f42644cdb07dc328383e 100644 (file)
--- a/lib/isc/netmgr/udp.c
+++ b/lib/isc/netmgr/udp.c
@@ -366,7 +366,11 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region,
 
        /*
         * Simulate a firewall blocking UDP packets bigger than
-        * 'maxudp' bytes.
+        * 'maxudp' bytes, for testing purposes.
+        *
+        * The client would ordinarily have unreferenced the handle
+        * in the callback, but that won't happen in this case, so
+        * we need to do so here.
         */
        if (maxudp != 0 && region->length > maxudp) {
                isc_nmhandle_unref(handle);
@@ -379,12 +383,10 @@ isc__nm_udp_send(isc_nmhandle_t *handle, isc_region_t *region,
        } else if (sock->type == isc_nm_udplistener) {
                psock = sock;
        } else {
-               isc_nmhandle_unref(handle);
                return (ISC_R_UNEXPECTED);
        }
 
        if (!isc__nmsocket_active(sock)) {
-               isc_nmhandle_unref(handle);
                return (ISC_R_CANCELED);
        }
 

diff --git a/lib/ns/client.c b/lib/ns/client.c
index 9ef8afa0944b7d3bf025745523b79e9c23cefe32..b60db14c80814f4fea8b0c5c40580b9835ab1be7 100644 (file)
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -381,8 +381,9 @@ ns_client_sendraw(ns_client_t *client, dns_message_t *message) {
        r.base[1] = client->message->id & 0xff;
 
        result = client_sendpkg(client, &buffer);
-       if (result == ISC_R_SUCCESS)
+       if (result == ISC_R_SUCCESS) {
                return;
+       }
 
  done:
        if (client->tcpbuf != NULL) {
@@ -392,6 +393,7 @@ ns_client_sendraw(ns_client_t *client, dns_message_t *message) {
        }
 
        ns_client_drop(client, result);
+       isc_nmhandle_unref(client->handle);
 }
 
 void
@@ -596,6 +598,10 @@ ns_client_send(ns_client_t *client) {
 
                isc_nmhandle_ref(client->handle);
                result = client_sendpkg(client, &tcpbuffer);
+               if (result != ISC_R_SUCCESS) {
+                       /* We won't get a callback to clean it up */
+                       isc_nmhandle_unref(client->handle);
+               }
 
                switch (isc_sockaddr_pf(&client->peeraddr)) {
                case AF_INET:
@@ -629,6 +635,10 @@ ns_client_send(ns_client_t *client) {
 
                isc_nmhandle_ref(client->handle);
                result = client_sendpkg(client, &buffer);
+               if (result != ISC_R_SUCCESS) {
+                       /* We won't get a callback to clean it up */
+                       isc_nmhandle_unref(client->handle);
+               }
 
                switch (isc_sockaddr_pf(&client->peeraddr)) {
                case AF_INET:

diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c
index 9c393c63acd2792ecead7b17b95c8c1eb9c2e3ec..7ad4ef96ea7eedf6aad8cc7f0ccf5982ab248533 100644 (file)
--- a/lib/ns/xfrout.c
+++ b/lib/ns/xfrout.c
@@ -1663,8 +1663,6 @@ xfrout_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
                xfrout_fail(xfr, result, "send");
        } else if (xfr->end_of_stream == false) {
                sendstream(xfr);
-               /* Return now so we don't unref the handle */
-               return;
        } else {
                /* End of zone transfer stream. */
                uint64_t msecs, persec;
@@ -1690,9 +1688,9 @@ xfrout_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
                           (unsigned int) persec);
 
                xfrout_ctx_destroy(&xfr);
+               /* We're done, unreference the handle */
+               isc_nmhandle_unref(handle);
        }
-
-       isc_nmhandle_unref(handle);
 }
 
 static void