lock find when unlinking adbname->finds in dns_adb_cancelfind()

In dns_adb_cancelfind(), we need to release the find lock and
then acquire the bucket and find locks in that order, for
consistency with locking hierarchy elsehwere. Previously we
were only acquiring the bucket lock.

Also rewrote the function for better readability.

diff --git a/lib/dns/adb.c b/lib/dns/adb.c
index dd311ee495b1d8e23901b5a48ea10a23f546cc5d..4a769f36351b85a2cd0a4f8e75c94151abc09192 100644 (file)
--- a/lib/dns/adb.c
+++ b/lib/dns/adb.c
@@ -2662,54 +2662,66 @@ dns_adb_destroyfind(dns_adbfind_t **findp) {
        free_adbfind(&find);
 }
 
+/*
+ * Caller must hold find lock.
+ */
+static void
+find_sendevent(dns_adbfind_t *find) {
+       if (!FIND_EVENTSENT(find)) {
+               isc_event_t *ev = &find->event;
+               isc_task_t *task = ev->ev_sender;
+
+               ev->ev_sender = find;
+               ev->ev_type = DNS_EVENT_ADBCANCELED;
+               ev->ev_destroy = event_freefind;
+               ev->ev_destroy_arg = find;
+               find->result_v4 = ISC_R_CANCELED;
+               find->result_v6 = ISC_R_CANCELED;
+
+               DP(DEF_LEVEL, "sending event %p to task %p for find %p", ev,
+                  task, find);
+
+               isc_task_sendanddetach(&task, (isc_event_t **)&ev);
+       }
+}
+
 void
 dns_adb_cancelfind(dns_adbfind_t *find) {
-       isc_event_t *ev = NULL;
-       isc_task_t *task = NULL;
-       dns_adb_t *adb = NULL;
        dns_adbname_t *adbname = NULL;
-       dns_adbnamebucket_t *nbucket = NULL;
-
-       LOCK(&find->lock);
 
        DP(DEF_LEVEL, "dns_adb_cancelfind on find %p", find);
 
-       adb = find->adb;
-       REQUIRE(DNS_ADB_VALID(adb));
+       REQUIRE(DNS_ADBFIND_VALID(find));
+       REQUIRE(DNS_ADB_VALID(find->adb));
 
+       LOCK(&find->lock);
        REQUIRE(!FIND_EVENTFREED(find));
        REQUIRE(FIND_WANTEVENT(find));
 
-       if (find->adbname == NULL) {
-               goto cleanup;
-       }
        adbname = find->adbname;
-       find->adbname = NULL;
-       nbucket = adbname->bucket;
 
-       UNLOCK(&find->lock);
-       LOCK(&nbucket->lock);
-       ISC_LIST_UNLINK(adbname->finds, find, plink);
-       UNLOCK(&nbucket->lock);
-       LOCK(&find->lock);
-
-cleanup:
-       if (!FIND_EVENTSENT(find)) {
-               ev = &find->event;
-               task = ev->ev_sender;
-               ev->ev_sender = find;
-               ev->ev_type = DNS_EVENT_ADBCANCELED;
-               ev->ev_destroy = event_freefind;
-               ev->ev_destroy_arg = find;
-               find->result_v4 = ISC_R_CANCELED;
-               find->result_v6 = ISC_R_CANCELED;
+       if (adbname == NULL) {
+               find_sendevent(find);
+               UNLOCK(&find->lock);
+       } else {
+               /*
+                * Release the find lock, then acquire the bucket and find
+                * locks in that order, to match locking hierarchy
+                * elsewhere.
+                */
+               UNLOCK(&find->lock);
+               LOCK(&adbname->bucket->lock);
+               LOCK(&find->lock);
 
-               DP(DEF_LEVEL, "sending event %p to task %p for find %p", ev,
-                  task, find);
+               if (find->adbname != NULL) {
+                       ISC_LIST_UNLINK(adbname->finds, find, plink);
+                       find->adbname = NULL;
+               }
+               find_sendevent(find);
 
-               isc_task_sendanddetach(&task, (isc_event_t **)&ev);
+               UNLOCK(&find->lock);
+               UNLOCK(&adbname->bucket->lock);
        }
-       UNLOCK(&find->lock);
 }
 
 void