4338. [bug] Reimplement change 4324 as it wasn't properly doing

                        all the required book keeping. [RT #41941]

diff --git a/CHANGES b/CHANGES
index 9ae68b39ca1c0399debaef0a2ee06354b343d407..322ac635032181e82aff1724074e20a52b967034 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+4338.  [bug]           Reimplement change 4324 as it wasn't properly doing
+                       all the required book keeping. [RT #41941]
+
 4337.  [bug]           The previous change exposed a latent flaw in
                        key refresh queries for managed-keys when
                        a cached DNSKEY had TTL 0. [RT #41986]

diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c
index 85b6b295934b37922ca242cd15ed261f1a4640e9..1e2257bb43a58ee2bc4cad2d67e01f6cef1abd52 100644 (file)
--- a/lib/dns/rbt.c
+++ b/lib/dns/rbt.c
@@ -2114,91 +2114,29 @@ dns_rbt_deletenode(dns_rbt_t *rbt, dns_rbtnode_t *node, isc_boolean_t recurse)
                }
        }
 
-       for (;;) {
-               /*
-                * Note the node that points to the level of the node
-                * that is being deleted.  If the deleted node is the
-                * top level, parent will be set to NULL.
-                */
-               parent = get_upper_node(node);
+       /*
+        * Note the node that points to the level of the node
+        * that is being deleted.  If the deleted node is the
+        * top level, parent will be set to NULL.
+        */
+       parent = get_upper_node(node);
 
-               /*
-                * This node now has no down pointer (either because it didn't
-                * have one to start, or because it was recursively removed).
-                * So now the node needs to be removed from this level.
-                */
-               deletefromlevel(node,
-                               parent == NULL ? &rbt->root : &DOWN(parent));
+       /*
+        * This node now has no down pointer, so now it needs
+        * to be removed from this level.
+        */
+       deletefromlevel(node, parent == NULL ? &rbt->root : &DOWN(parent));
 
-               if (DATA(node) != NULL && rbt->data_deleter != NULL)
-                       rbt->data_deleter(DATA(node), rbt->deleter_arg);
+       if (DATA(node) != NULL && rbt->data_deleter != NULL)
+               rbt->data_deleter(DATA(node), rbt->deleter_arg);
 
-               unhash_node(rbt, node);
+       unhash_node(rbt, node);
 #if DNS_RBT_USEMAGIC
-               node->magic = 0;
+       node->magic = 0;
 #endif
-               dns_rbtnode_refdestroy(node);
-
-               freenode(rbt, &node);
-
-               /*
-                * There are now two special cases that can exist that
-                * would not have existed if the tree had been created
-                * using only the names that now exist in it.  (This is
-                * all related to join_nodes() as described in this
-                * function's introductory comment.)  Both cases exist
-                * when the deleted node's parent (the node that pointed
-                * to the deleted node's level) is not null but it has
-                * no data: parent != NULL && DATA(parent) == NULL.
-                *
-                * The first case is that the deleted node was the last
-                * on its level: DOWN(parent) == NULL.  This case can
-                * only exist if the parent was previously deleted --
-                * and so now, apparently, the parent should go away.
-                * That can't be done though because there might be
-                * external references to it, such as through a
-                * nodechain. XXXMUKS: We don't care about this.
-                *
-                * The other case also involves a parent with no data,
-                * but with the deleted node being the next-to-last node
-                * instead of the last: LEFT(DOWN(parent)) == NULL &&
-                * RIGHT(DOWN(parent)) == NULL.  Presumably now the
-                * remaining node on the level should be joined with the
-                * parent, but it's already been described why that
-                * can't be done.
-                */
-
-               /*
-                * Is this the root?.
-                */
-               if (parent == NULL)
-                       break;
+       dns_rbtnode_refdestroy(node);
 
-               /*
-                * If the node deletion did not cause the subtree to disappear
-                * completely, return early.
-                */
-               if (DOWN(parent) != NULL)
-                       break;
-
-               /*
-                * If the upper node is not empty, it cannot be deleted.
-                */
-               if (DATA(parent) != NULL)
-                       break;
-
-               /*
-                * If upper node is the root node (.), don't attempt to
-                * delete it. The root node must always exist.
-                */
-               if (parent == rbt->root)
-                       break;
-
-               /*
-                * Ascend up the tree and delete the upper node.
-                */
-               node = parent;
-       }
+       freenode(rbt, &node);
 
        /*
         * This function never fails.

diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index ee1e4b5ec567ad6ef2912bacbcae28c789929796..d831162001f43913f4e4d0576eaecd1884235762 100644 (file)
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -1977,6 +1977,24 @@ delete_node(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node) {
        }
 }
 
+/*
+ * Caller must be holding the node lock.
+ */
+static inline void
+new_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node) {
+       unsigned int lockrefs, noderefs;
+       isc_refcount_t *lockref;
+
+       INSIST(!ISC_LINK_LINKED(node, deadlink));
+       dns_rbtnode_refincrement0(node, &noderefs);
+       if (noderefs == 1) {    /* this is the first reference to the node */
+               lockref = &rbtdb->node_locks[node->locknum].references;
+               isc_refcount_increment0(lockref, &lockrefs);
+               INSIST(lockrefs != 0);
+       }
+       INSIST(noderefs != 0);
+}
+
 /*%
  * Clean up dead nodes.  These are nodes which have no references, and
  * have no data.  They are dead but we could not or chose not to delete
@@ -2001,31 +2019,35 @@ cleanup_dead_nodes(dns_rbtdb_t *rbtdb, int bucketnum) {
                INSIST(dns_rbtnode_refcurrent(node) == 0 &&
                       node->data == NULL);
 
-               delete_node(rbtdb, node);
+               if (node->parent != NULL &&
+                   node->parent->down == node && node->left == NULL &&
+                   node->right == NULL && rbtdb->task != NULL)
+               {
+                       isc_event_t *ev;
+                       dns_db_t *db;
 
+                       ev = isc_event_allocate(rbtdb->common.mctx, NULL,
+                                               DNS_EVENT_RBTPRUNE,
+                                               prune_tree, node,
+                                               sizeof(isc_event_t));
+                       if (ev != NULL) {
+                               new_reference(rbtdb, node);
+                               db = NULL;
+                               attach((dns_db_t *)rbtdb, &db);
+                               ev->ev_sender = db;
+                               isc_task_send(rbtdb->task, &ev);
+                       } else {
+                               ISC_LIST_APPEND(rbtdb->deadnodes[bucketnum],
+                                               node, deadlink);
+                       }
+               } else {
+                       delete_node(rbtdb, node);
+               }
                node = ISC_LIST_HEAD(rbtdb->deadnodes[bucketnum]);
                count--;
        }
 }
 
-/*
- * Caller must be holding the node lock.
- */
-static inline void
-new_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node) {
-       unsigned int lockrefs, noderefs;
-       isc_refcount_t *lockref;
-
-       INSIST(!ISC_LINK_LINKED(node, deadlink));
-       dns_rbtnode_refincrement0(node, &noderefs);
-       if (noderefs == 1) {    /* this is the first reference to the node */
-               lockref = &rbtdb->node_locks[node->locknum].references;
-               isc_refcount_increment0(lockref, &lockrefs);
-               INSIST(lockrefs != 0);
-       }
-       INSIST(noderefs != 0);
-}
-
 /*
  * This function is assumed to be called when a node is newly referenced
  * and can be in the deadnode list.  In that case the node must be retrieved