Attachment 597049 Details for Bug 837165 – Proposed patch

[patch] Proposed patch

bind9.git-e1c47cd778c7be1aa5b4eabc9c94d29eb1c8baa5.patch (text/plain), 6.13 KB, created by Adam Tkac on 2012-07-09 11:51:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Adam Tkac

Created: 2012-07-09 11:51:34 UTC

Size: 6.13 KB

patch

obsolete

>From e1c47cd778c7be1aa5b4eabc9c94d29eb1c8baa5 Mon Sep 17 00:00:00 2001
>From: Mark Andrews <marka@isc.org>
>Date: Wed, 15 Feb 2012 02:02:55 +0000
>Subject: [PATCH 1/1] 3284.   [bug]           Address race conditions with the
> handling of                         rbtnode.deadlink.
> [RT #27738]
>
>---
> CHANGES         |    3 ++
> lib/dns/rbtdb.c |   91 ++++++++++++++++++++++++++++---------------------------
> 2 files changed, 49 insertions(+), 45 deletions(-)
>
>diff --git a/CHANGES b/CHANGES
>index f0362c1..16a4fe8 100644
>--- a/CHANGES
>+++ b/CHANGES
>@@ -1,3 +1,6 @@
>+3284.	[bug]		Address race conditions with the handling of
>+			rbtnode.deadlink. [RT #27738]
>+
> 3283.	[bug]		Raw zones with with more than 512 records in a RRset
> 			failed to load. [RT #27863]
> 
>diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
>index d2d7e07..470d168 100644
>--- a/lib/dns/rbtdb.c
>+++ b/lib/dns/rbtdb.c
>@@ -15,7 +15,7 @@
>  * PERFORMANCE OF THIS SOFTWARE.
>  */
> 
>-/* $Id: rbtdb.c,v 1.310.8.17 2012/02/09 23:46:25 tbox Exp $ */
>+/* $Id: rbtdb.c,v 1.310.8.18 2012/02/15 02:02:55 marka Exp $ */
> 
> /*! \file */
> 
>@@ -1608,8 +1608,7 @@ cleanup_dead_nodes(dns_rbtdb_t *rbtdb, int bucketnum) {
> }
> 
> /*
>- * Caller must be holding the node lock if its reference must be protected
>- * by the lock.
>+ * Caller must be holding the node lock.
>  */
> static inline void
> new_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node) {
>@@ -1640,31 +1639,40 @@ static inline void
> reactivate_node(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
> 		isc_rwlocktype_t treelocktype)
> {
>-	isc_boolean_t need_relock = ISC_FALSE;
>+	isc_rwlocktype_t locktype = isc_rwlocktype_read;
>+	nodelock_t *nodelock = &rbtdb->node_locks[node->locknum].lock;
>+	isc_boolean_t maybe_cleanup = ISC_FALSE;
> 
>-	NODE_STRONGLOCK(&rbtdb->node_locks[node->locknum].lock);
>-	NODE_WEAKLOCK(&rbtdb->node_locks[node->locknum].lock,
>-		      isc_rwlocktype_read);
>-	if (ISC_LINK_LINKED(node, deadlink))
>-		need_relock = ISC_TRUE;
>-	else if (!ISC_LIST_EMPTY(rbtdb->deadnodes[node->locknum]) &&
>-		 treelocktype == isc_rwlocktype_write)
>-		need_relock = ISC_TRUE;
>-	NODE_WEAKUNLOCK(&rbtdb->node_locks[node->locknum].lock,
>-			isc_rwlocktype_read);
>-	if (need_relock) {
>-		NODE_WEAKLOCK(&rbtdb->node_locks[node->locknum].lock,
>-			      isc_rwlocktype_write);
>+	NODE_STRONGLOCK(nodelock);
>+	NODE_WEAKLOCK(nodelock, locktype);
>+
>+	/*
>+	 * Check if we can possibly cleanup the dead node.  If so, upgrade
>+	 * the node lock below to perform the cleanup.
>+	 */
>+	if (!ISC_LIST_EMPTY(rbtdb->deadnodes[node->locknum]) &&
>+	    treelocktype == isc_rwlocktype_write) {
>+		maybe_cleanup = ISC_TRUE;
>+	}
>+
>+	if (ISC_LINK_LINKED(node, deadlink) || maybe_cleanup) {
>+		/*
>+		 * Upgrade the lock and test if we still need to unlink.
>+		 */
>+		NODE_WEAKUNLOCK(nodelock, locktype);
>+		locktype = isc_rwlocktype_write;
>+		NODE_WEAKLOCK(nodelock, locktype);
> 		if (ISC_LINK_LINKED(node, deadlink))
> 			ISC_LIST_UNLINK(rbtdb->deadnodes[node->locknum],
> 					node, deadlink);
>-		if (treelocktype == isc_rwlocktype_write)
>+		if (maybe_cleanup)
> 			cleanup_dead_nodes(rbtdb, node->locknum);
>-		NODE_WEAKUNLOCK(&rbtdb->node_locks[node->locknum].lock,
>-				isc_rwlocktype_write);
> 	}
>+
> 	new_reference(rbtdb, node);
>-	NODE_STRONGUNLOCK(&rbtdb->node_locks[node->locknum].lock);
>+
>+	NODE_WEAKUNLOCK(nodelock, locktype);
>+	NODE_STRONGUNLOCK(nodelock);
> }
> 
> /*
>@@ -1688,7 +1696,7 @@ decrement_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
> 	rbtdb_nodelock_t *nodelock;
> 	unsigned int refs, nrefs;
> 	int bucket = node->locknum;
>-	isc_boolean_t no_reference;
>+	isc_boolean_t no_reference = ISC_TRUE;
> 
> 	nodelock = &rbtdb->node_locks[bucket];
> 
>@@ -1708,6 +1716,7 @@ decrement_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
> 		NODE_WEAKUNLOCK(&nodelock->lock, isc_rwlocktype_read);
> 		NODE_WEAKLOCK(&nodelock->lock, isc_rwlocktype_write);
> 	}
>+
> 	dns_rbtnode_refdecrement(node, &nrefs);
> 	INSIST((int)nrefs >= 0);
> 	if (nrefs > 0) {
>@@ -1717,7 +1726,7 @@ decrement_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
> 		return (ISC_FALSE);
> 	}
> 
>-	if (node->dirty && dns_rbtnode_refcurrent(node) == 0) {
>+	if (node->dirty) {
> 		if (IS_CACHE(rbtdb))
> 			clean_cache_node(rbtdb, node);
> 		else {
>@@ -1735,19 +1744,6 @@ decrement_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
> 		}
> 	}
> 
>-	isc_refcount_decrement(&nodelock->references, &refs);
>-	INSIST((int)refs >= 0);
>-
>-	/*
>-	 * XXXDCL should this only be done for cache zones?
>-	 */
>-	if (node->data != NULL || node->down != NULL) {
>-		/* Restore the lock? */
>-		if (nlock == isc_rwlocktype_read)
>-			NODE_WEAKDOWNGRADE(&nodelock->lock);
>-		return (ISC_TRUE);
>-	}
>-
> 	/*
> 	 * Attempt to switch to a write lock on the tree.  If this fails,
> 	 * we will add this node to a linked list of nodes in this locking
>@@ -1771,13 +1767,18 @@ decrement_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
> 	} else
> 		write_locked = ISC_TRUE;
> 
>-	no_reference = ISC_TRUE;
>-	if (write_locked && dns_rbtnode_refcurrent(node) == 0) {
>+	isc_refcount_decrement(&nodelock->references, &refs);
>+	INSIST((int)refs >= 0);
>+
>+	/*
>+	 * XXXDCL should this only be done for cache zones?
>+	 */
>+	if (node->data != NULL || node->down != NULL)
>+		goto restore_locks;
>+
>+	if (write_locked) {
> 		/*
>-		 * We can now delete the node if the reference counter is
>-		 * zero.  This should be typically the case, but a different
>-		 * thread may still gain a (new) reference just before the
>-		 * current thread locks the tree (e.g., in findnode()).
>+		 * We can now delete the node.
> 		 */
> 
> 		/*
>@@ -1852,13 +1853,13 @@ decrement_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
> 
> 			delete_node(rbtdb, node);
> 		}
>-	} else if (dns_rbtnode_refcurrent(node) == 0) {
>+	} else {
> 		INSIST(node->data == NULL);
> 		INSIST(!ISC_LINK_LINKED(node, deadlink));
> 		ISC_LIST_APPEND(rbtdb->deadnodes[bucket], node, deadlink);
>-	} else
>-		no_reference = ISC_FALSE;
>+	}
> 
>+ restore_locks:
> 	/* Restore the lock? */
> 	if (nlock == isc_rwlocktype_read)
> 		NODE_WEAKDOWNGRADE(&nodelock->lock);
>-- 
>1.7.9.2
>
></TD></TR></TABLE>
><DIV class="copy">
><TABLE CLASS="copyright">
><TR CLASS="copyright"><TD VALIGN="left" CLASS="copyright">
>Copyright &copy; 2004, 2005, 2006 ISC, Inc.
></TD>
><TD CLASS="copyright" ALIGN="right"> 2012-07-09 11:43:19</TD>
></TR>
></TABLE>
></DIV>
></BODY>
></HTML>

Actions: View | Diff

Attachments on bug 837165: 597049