configuration locking checkpoint

bin/named/client.c

@@ -34,6 +34,7 @@
 #include <dns/rdataset.h>
 #include <dns/view.h>
 #include <dns/xfrin.h>
+#include <dns/zone.h>
 
 #include <named/globals.h>
 #include <named/client.h>
@@ -361,7 +362,23 @@ client_senddone(isc_task_t *task, isc_event_t *event) {
 	 */
 	if (client->waiting_for_bufs == ISC_TRUE) {
 		client->waiting_for_bufs = ISC_FALSE;
+
+		/*
+		 * Must lock the view here because ns_client_send()
+		 * uses view configuration for TSIGs and stuff.
+		 */
+		RWLOCK(&ns_g_server->conflock, isc_rwlocktype_read);
+		dns_zonemgr_lockconf(ns_g_server->zonemgr, isc_rwlocktype_read);
+		dns_view_attach(client->view, &client->lockview);
+		RWLOCK(&client->lockview->conflock, isc_rwlocktype_read);
+		
 		ns_client_send(client);
+		
+		RWUNLOCK(&client->lockview->conflock, isc_rwlocktype_read);
+		dns_view_detach(&client->lockview);
+		dns_zonemgr_unlockconf(ns_g_server->zonemgr, isc_rwlocktype_read);
+		RWUNLOCK(&ns_g_server->conflock, isc_rwlocktype_read);
+
 		return;
 	}
 	/* XXXRTH need to add exit draining mode. */
@@ -603,6 +620,9 @@ client_request(isc_task_t *task, isc_event_t *event) {
 	REQUIRE(task == client->task);
 
 	INSIST(client->recursionquota == NULL);
+
+	RWLOCK(&ns_g_server->conflock, isc_rwlocktype_read);
+	dns_zonemgr_lockconf(ns_g_server->zonemgr, isc_rwlocktype_read);
 	
 	if (event->type == DNS_EVENT_DISPATCH) {
 		devent = (dns_dispatchevent_t *)event;
@@ -623,7 +643,7 @@ client_request(isc_task_t *task, isc_event_t *event) {
 
 	if (client->shuttingdown) {
 		maybe_free(client);
-		return;
+		goto cleanup_serverlock;
 	}
 		
 	isc_stdtime_get(&client->requesttime);
@@ -634,13 +654,13 @@ client_request(isc_task_t *task, isc_event_t *event) {
 			ns_client_next(client, result);
 		else
 			isc_task_shutdown(client->task);
-		return;
+		goto cleanup_serverlock;
 	}
 
 	result = dns_message_parse(client->message, buffer, ISC_FALSE);
 	if (result != ISC_R_SUCCESS) {
 		ns_client_error(client, result);
-		return;
+		goto cleanup_serverlock;
 	}
 
 	/*
@@ -651,7 +671,7 @@ client_request(isc_task_t *task, isc_event_t *event) {
 	if ((client->message->flags & DNS_MESSAGEFLAG_QR) != 0) {
 		CTRACE("unexpected response");
 		ns_client_next(client, DNS_R_FORMERR);
-		return;
+		goto cleanup_serverlock;
 	}
 
 	/*
@@ -672,7 +692,7 @@ client_request(isc_task_t *task, isc_event_t *event) {
 		result = client_addopt(client);
 		if (result != ISC_R_SUCCESS) {
 			ns_client_error(client, result);
-			return;
+			goto cleanup_serverlock;
 		}
 
 		/*
@@ -683,7 +703,7 @@ client_request(isc_task_t *task, isc_event_t *event) {
 		version = (opt->ttl & 0x00FF0000) >> 16;
 		if (version != 0) {
 			ns_client_error(client, DNS_R_BADVERS);
-			return;
+			goto cleanup_serverlock;
 		}
 	}
 
@@ -691,7 +711,6 @@ client_request(isc_task_t *task, isc_event_t *event) {
 	 * XXXRTH  View list management code will be moving to its own module
 	 *         soon.
 	 */
-	RWLOCK(&ns_g_server->viewlock, isc_rwlocktype_read);
 	for (view = ISC_LIST_HEAD(ns_g_server->viewlist);
 	     view != NULL;
 	     view = ISC_LIST_NEXT(view, link)) {
@@ -705,14 +724,25 @@ client_request(isc_task_t *task, isc_event_t *event) {
 			break;
 		}
 	}
-	RWUNLOCK(&ns_g_server->viewlock, isc_rwlocktype_read);
 
 	if (view == NULL) {
 		CTRACE("no view");
 		ns_client_error(client, DNS_R_REFUSED);
-		return;
+		goto cleanup_serverlock;
 	}
 
+	/*
+	 * Lock the view's configuration data for reading.
+	 * We must attach a separate view reference for this
+	 * purpose instad of using client->view, because
+	 * client->view may or may not be detached at the point
+	 * when whe return from this event handler depending
+	 * on whether the request handler causes ns_client_next()
+	 * to be called or not.
+	 */
+	dns_view_attach(client->view, &client->lockview);
+	RWLOCK(&client->lockview->conflock, isc_rwlocktype_read);
+
 	/*
 	 * Check for a signature.  We log bad signatures regardless of 
 	 * whether they ultimately cause the request to be rejected or
@@ -722,7 +752,7 @@ client_request(isc_task_t *task, isc_event_t *event) {
 	result = dns_message_checksig(client->message, client->view);
 	if (result != ISC_R_SUCCESS) {
 		ns_client_error(client, result);
-		return;
+		goto cleanup_viewlock;
 	}
 
 	client->signer = NULL;
@@ -797,6 +827,13 @@ client_request(isc_task_t *task, isc_event_t *event) {
 		CTRACE("unknown opcode");
 		ns_client_error(client, DNS_R_NOTIMP);
 	}
+
+ cleanup_viewlock:
+	RWUNLOCK(&client->lockview->conflock, isc_rwlocktype_read);
+	dns_view_detach(&client->lockview);
+ cleanup_serverlock:
+	dns_zonemgr_unlockconf(ns_g_server->zonemgr, isc_rwlocktype_read);
+	RWUNLOCK(&ns_g_server->conflock, isc_rwlocktype_read);
 }
 
 static void
@@ -882,6 +919,7 @@ client_create(ns_clientmgr_t *manager,
 	client->nwaiting = 0;
 	client->attributes = 0;
 	client->view = NULL;
+	client->lockview = NULL;
 	client->dispatch = NULL;
 	client->dispentry = NULL;
 	client->dispevent = NULL;

bin/named/include/named/client.h

@@ -89,6 +89,7 @@ struct ns_client {
 	unsigned int			attributes;
 	isc_task_t *			task;
 	dns_view_t *			view;
+	dns_view_t *			lockview;
 	dns_dispatch_t *		dispatch;
 	dns_dispentry_t *		dispentry;
 	dns_dispatchevent_t *		dispevent;

bin/named/include/named/server.h

@@ -35,6 +35,9 @@ struct ns_server {
 	isc_mem_t *		mctx;
 
 	isc_task_t *		task;
+
+	/* Common rwlock for the server's configurable data. */
+	isc_rwlock_t		conflock;
 	
 	/* Configurable data. */
 	isc_boolean_t		recursion;
@@ -51,7 +54,6 @@ struct ns_server {
 	dns_zonemgr_t *		zonemgr;
 	ns_clientmgr_t *	clientmgr;
 	dns_viewlist_t		viewlist;
-	isc_rwlock_t		viewlock;
 	ns_interfacemgr_t *	interfacemgr;
 	dns_db_t *		roothints;
 	dns_tkey_ctx_t *	tkeyctx;

bin/named/query.c

@@ -1681,7 +1681,18 @@ query_resume(isc_task_t *task, isc_event_t *event) {
 		ns_client_unwait(client);
 	} else {
 		ns_client_unwait(client);
+
+		RWLOCK(&ns_g_server->conflock, isc_rwlocktype_read);
+		dns_zonemgr_lockconf(ns_g_server->zonemgr, isc_rwlocktype_read);
+		dns_view_attach(client->view, &client->lockview);
+		RWLOCK(&client->lockview->conflock, isc_rwlocktype_read);
+
 		query_find(client, devent);
+		
+		RWUNLOCK(&client->lockview->conflock, isc_rwlocktype_read);
+		dns_view_detach(&client->lockview);		
+		dns_zonemgr_unlockconf(ns_g_server->zonemgr, isc_rwlocktype_read);
+		RWUNLOCK(&ns_g_server->conflock, isc_rwlocktype_read);
 	}
 }
 

bin/named/server.c

@@ -131,6 +131,8 @@ configure_view(dns_view_t *view, dns_c_ctx_t *cctx, isc_mem_t *mctx,
 
 	REQUIRE(DNS_VIEW_VALID(view));
 
+	RWLOCK(&view->conflock, isc_rwlocktype_write);
+	
 	/*
 	 * Cache.
 	 */
@@ -177,6 +179,8 @@ configure_view(dns_view_t *view, dns_c_ctx_t *cctx, isc_mem_t *mctx,
 	dns_view_setkeyring(view, ring);
 
  cleanup:
+	RWUNLOCK(&view->conflock, isc_rwlocktype_write);
+	
 	return (result);
 }
 
@@ -274,7 +278,7 @@ create_version_view(dns_c_ctx_t *configctx, dns_view_t **viewp) {
  * is called after parsing each "zone" statement in named.conf.
  */
 static isc_result_t
-load_zone(dns_c_ctx_t *cctx, dns_c_zone_t *czone, dns_c_view_t *cview,
+configure_zone(dns_c_ctx_t *cctx, dns_c_zone_t *czone, dns_c_view_t *cview,
 	  void *uap)
 {
 	ns_load_t *lctx = (ns_load_t *) uap;
@@ -290,7 +294,7 @@ load_zone(dns_c_ctx_t *cctx, dns_c_zone_t *czone, dns_c_view_t *cview,
 	isc_buffer_t buffer;
 	dns_fixedname_t fixorigin;
 	dns_name_t *origin;
-	
+
 	/*
 	 * Get the zone origin as a dns_name_t.
 	 */
@@ -345,11 +349,9 @@ load_zone(dns_c_ctx_t *cctx, dns_c_zone_t *czone, dns_c_view_t *cview,
 	 *     options (e.g., an existing master zone cannot 
 	 *     be reused if the options specify a slave zone)
 	 */
-	RWLOCK(&ns_g_server->viewlock, isc_rwlocktype_read);
 	result = dns_viewlist_find(&ns_g_server->viewlist,
 				   view->name, view->rdclass,
 				   &pview);
-     	RWUNLOCK(&ns_g_server->viewlock, isc_rwlocktype_read);
 	if (result != ISC_R_NOTFOUND && result != ISC_R_SUCCESS)
 		goto cleanup;
 	if (pview != NULL)
@@ -530,11 +532,14 @@ load_configuration(const char *filename, ns_server_t *server) {
 
 	dns_aclconfctx_init(&aclconfctx);
 
+	RWLOCK(&server->conflock, isc_rwlocktype_write);
+	dns_zonemgr_lockconf(server->zonemgr, isc_rwlocktype_write);
+	
 	lctx.mctx = ns_g_mctx;
 	lctx.aclconf = &aclconfctx;
 	ISC_LIST_INIT(lctx.viewlist);
 
-	callbacks.zonecbk = load_zone;
+	callbacks.zonecbk = configure_zone;
 	callbacks.zonecbkuap = &lctx;
 	callbacks.optscbk = NULL;
 	callbacks.optscbkuap = NULL;
@@ -546,7 +551,7 @@ load_configuration(const char *filename, ns_server_t *server) {
 	/*
 	 * Parse the configuration file creating a parse tree.  Any
 	 * 'zone' statements are handled immediately by calling
-	 * load_zone() through 'callbacks'.
+	 * configure_zone() through 'callbacks'.
 	 */
 	configctx = NULL;
 	CHECK(dns_c_parse_namedconf(filename, ns_g_mctx, &configctx,
@@ -673,11 +678,9 @@ load_configuration(const char *filename, ns_server_t *server) {
 	/*
 	 * Swap our new view list with the production one.
 	 */
-	RWLOCK(&server->viewlock, isc_rwlocktype_write);
 	tmpviewlist = server->viewlist;
 	server->viewlist = lctx.viewlist;
 	lctx.viewlist = tmpviewlist;
-	RWUNLOCK(&server->viewlock, isc_rwlocktype_write);
 
 	/*
 	 * Load the TKEY information from the configuration.
@@ -707,7 +710,11 @@ load_configuration(const char *filename, ns_server_t *server) {
 		view_next = ISC_LIST_NEXT(view, link);
 		ISC_LIST_UNLINK(lctx.viewlist, view, link);
 		dns_view_detach(&view);
+
 	}
+
+	dns_zonemgr_unlockconf(server->zonemgr, isc_rwlocktype_write);
+	RWUNLOCK(&server->conflock, isc_rwlocktype_write);	
 	return (result);
 }
 
@@ -715,6 +722,8 @@ static isc_result_t
 load_zones(ns_server_t *server, isc_boolean_t stop) {
 	isc_result_t result;
 	dns_view_t *view;
+
+	dns_zonemgr_lockconf(server->zonemgr, isc_rwlocktype_read);
 	
 	/*
 	 * Load zone data from disk.
@@ -733,6 +742,7 @@ load_zones(ns_server_t *server, isc_boolean_t stop) {
 	 */
 	CHECK(dns_zonemgr_forcemaint(server->zonemgr));
  cleanup:
+	dns_zonemgr_unlockconf(server->zonemgr, isc_rwlocktype_read);	
 	return (result);
 }
 
@@ -768,13 +778,13 @@ shutdown_server(isc_task_t *task, isc_event_t *event) {
 	dns_view_t *view, *view_next;
 	ns_server_t *server = (ns_server_t *) event->arg;
 		
-	(void)task;
+	UNUSED(task);
+
+	RWLOCK(&server->conflock, isc_rwlocktype_write);
 
 	isc_log_write(ns_g_lctx, NS_LOGCATEGORY_GENERAL, NS_LOGMODULE_SERVER,
 		      ISC_LOG_INFO, "shutting down");
 
-	RWLOCK(&server->viewlock, isc_rwlocktype_write);
-
 	for (view = ISC_LIST_HEAD(server->viewlist);
 	     view != NULL;
 	     view = view_next) {
@@ -783,17 +793,16 @@ shutdown_server(isc_task_t *task, isc_event_t *event) {
 		dns_view_detach(&view);
 	}
 
-	RWUNLOCK(&server->viewlock, isc_rwlocktype_write);
-
-
 	ns_clientmgr_destroy(&server->clientmgr);
 	ns_interfacemgr_shutdown(server->interfacemgr);
 	ns_interfacemgr_detach(&server->interfacemgr);	
 	dns_zonemgr_shutdown(server->zonemgr);
 	
 	isc_task_detach(&server->task);
-	
+
 	isc_event_free(&event);
+
+	RWUNLOCK(&server->conflock, isc_rwlocktype_write);
 }
 
 void
@@ -807,11 +816,14 @@ ns_server_create(isc_mem_t *mctx, ns_server_t **serverp) {
 	server->mctx = mctx;
 	server->task = NULL;
 	
+	CHECKFATAL(isc_rwlock_init(&server->conflock, UINT_MAX, UINT_MAX),
+		   "initializing server configuration lock");
+
 	/* Initialize configuration data with default values. */
 	server->recursion = ISC_TRUE;
 	server->auth_nxdomain = ISC_FALSE; /* Was true in BIND 8 */
 	server->transfer_format = dns_one_answer;
-		
+
 	server->queryacl = NULL;
 	server->recursionacl = NULL;
 	server->transferacl = NULL;
@@ -828,8 +840,6 @@ ns_server_create(isc_mem_t *mctx, ns_server_t **serverp) {
 	server->clientmgr = NULL;
 	server->interfacemgr = NULL;
 	ISC_LIST_INIT(server->viewlist);
-	result = isc_rwlock_init(&server->viewlock, 0, 0);
-	RUNTIME_CHECK(result == ISC_R_SUCCESS); 	
 	server->roothints = NULL;
 		
 	CHECKFATAL(dns_rootns_create(mctx, &server->roothints),
@@ -888,8 +898,6 @@ ns_server_destroy(ns_server_t **serverp) {
 	
 	dns_db_detach(&server->roothints);
 	
-	isc_rwlock_destroy(&server->viewlock);
-	
 	if (server->queryacl != NULL)
 		dns_acl_detach(&server->queryacl);
 	if (server->recursionacl != NULL)
@@ -900,6 +908,7 @@ ns_server_destroy(ns_server_t **serverp) {
 	isc_quota_destroy(&server->recursionquota);
 	isc_quota_destroy(&server->tcpquota);
 	isc_quota_destroy(&server->xfroutquota);
+	isc_rwlock_destroy(&server->conflock);
 
 	server->magic = 0;
 	isc_mem_put(server->mctx, server, sizeof(*server));

lib/dns/include/dns/view.h

@@ -63,6 +63,7 @@
 #include <isc/lang.h>
 #include <isc/event.h>
 #include <isc/mutex.h>
+#include <isc/rwlock.h>
 #include <isc/stdtime.h>
 
 #include <dns/types.h>
@@ -84,10 +85,12 @@ struct dns_view {
 	dns_db_t *			hints;
 	dns_rbt_t *			secroots;
 	isc_mutex_t			lock;
+	isc_rwlock_t			conflock;
 	isc_boolean_t			frozen;
 	isc_task_t *			task;
 	isc_event_t			resevent;
 	isc_event_t			adbevent;
+	/* Configurable data, locked by conflock. */
 	dns_tsig_keyring_t *		statickeys;
 	dns_tsig_keyring_t *		dynamickeys;
 	/* Locked by lock. */

lib/dns/include/dns/zone.h

@@ -25,6 +25,7 @@
 #include <isc/lang.h>
 #include <isc/mem.h>
 #include <isc/mutex.h>
+#include <isc/rwlock.h>
 #include <isc/time.h>
 #include <isc/stdtime.h>
 #include <isc/socket.h>
@@ -798,6 +799,12 @@ dns_zonemgr_destroy(dns_zonemgr_t **zmgrp);
 void
 dns_zonemgr_releasezone(dns_zonemgr_t *zmgr, dns_zone_t *zone);
 
+void
+dns_zonemgr_lockconf(dns_zonemgr_t *zmgr, isc_rwlocktype_t type);
+
+void
+dns_zonemgr_unlockconf(dns_zonemgr_t *zmgr, isc_rwlocktype_t type);
+
 ISC_LANG_ENDDECLS
 
 #endif	/* DNS_ZONE_H */

lib/dns/view.c

@@ -77,6 +77,14 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass,
 		result = ISC_R_UNEXPECTED;
 		goto cleanup_name;
 	}
+	result = isc_rwlock_init(&view->conflock, UINT_MAX, UINT_MAX);
+	if (result != ISC_R_SUCCESS) {
+		UNEXPECTED_ERROR(__FILE__, __LINE__,
+				 "isc_rwlock_init() failed: %s",
+				 isc_result_totext(result));
+		result = ISC_R_UNEXPECTED;
+		goto cleanup_mutex;
+	}
 	view->zonetable = NULL;
 	result = dns_zt_create(mctx, rdclass, &view->zonetable);
 	if (result != ISC_R_SUCCESS) {
@@ -84,7 +92,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass,
 				 "dns_zt_create() failed: %s",
 				 isc_result_totext(result));
 		result = ISC_R_UNEXPECTED;
-		goto cleanup_mutex;
+		goto cleanup_rwlock;
 	}
 	view->secroots = NULL;
 	result = dns_rbt_create(mctx, NULL, NULL, &view->secroots);
@@ -128,6 +136,9 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass,
  cleanup_zt:
 	dns_zt_detach(&view->zonetable);
 
+ cleanup_rwlock:
+	isc_rwlock_destroy(&view->conflock);
+
  cleanup_mutex:
 	isc_mutex_destroy(&view->lock);
 

lib/dns/zone.c

@@ -15,7 +15,7 @@
  * SOFTWARE.
  */
 
- /* $Id: zone.c,v 1.64 2000/01/27 00:44:53 gson Exp $ */
+ /* $Id: zone.c,v 1.65 2000/01/27 01:00:10 gson Exp $ */
 
 #include <config.h>
 
@@ -25,7 +25,6 @@
 #include <isc/error.h>
 #include <isc/magic.h>
 #include <isc/print.h>
-#include <isc/rwlock.h>
 #include <isc/serial.h>
 #include <isc/taskpool.h>
 #include <isc/timer.h>
@@ -185,6 +184,7 @@ struct dns_zonemgr {
 	isc_taskpool_t *	zonetasks;
 	isc_task_t *		task;
 	isc_rwlock_t		rwlock;
+	isc_rwlock_t		conflock;
 	/* Locked by rwlock. */
 	ISC_LIST(dns_zone_t)	zones;
 };
@@ -2134,12 +2134,16 @@ static void
 zone_timer(isc_task_t *task, isc_event_t *event) {
 	const char me[] = "zone_timer";
 	dns_zone_t *zone = (dns_zone_t *)event->arg;
+	UNUSED(task);
 
 	DNS_ENTER;
 
+	dns_zonemgr_lockconf(zone->zmgr, isc_rwlocktype_read);
+	/* XXX if we use a view, we need to lock its configuration, too. */
 	dns_zone_maintenance(zone);
+	dns_zonemgr_unlockconf(zone->zmgr, isc_rwlocktype_read);
+	
 	isc_event_free(&event);
-	task = task; /* XXX */
 }
 
 static isc_result_t
@@ -2970,26 +2974,42 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr,
 		UNEXPECTED_ERROR(__FILE__, __LINE__,
 				 "isc_rwlock_init() failed: %s",
 				 isc_result_totext(result));
-		return (DNS_R_UNEXPECTED);
+		result = DNS_R_UNEXPECTED;
+		goto free_mem;
+	}
+	result = isc_rwlock_init(&zmgr->conflock, UINT_MAX, UINT_MAX);
+	if (result != ISC_R_SUCCESS) {
+		isc_mem_put(mctx, zmgr, sizeof *zmgr);
+		UNEXPECTED_ERROR(__FILE__, __LINE__,
+				 "isc_rwlock_init() failed: %s",
+				 isc_result_totext(result));
+		result = DNS_R_UNEXPECTED;
+		goto free_rwlock;
 	}
 
 	/* Create the zone task pool. */
 	result = isc_taskpool_create(taskmgr, mctx, 
 				     8 /* XXX */, 0, &zmgr->zonetasks);
 	if (result != ISC_R_SUCCESS)
-		goto failure;
+		goto free_conflock;
 
 	/* Create a single task for queueing of SOA queries. */
 	result = isc_task_create(taskmgr, mctx, 1, &zmgr->task);
 	if (result != ISC_R_SUCCESS)
-		goto failure;
+		goto free_taskpool;
 	isc_task_setname(zmgr->task, "zmgr", zmgr);
 
 	*zmgrp = zmgr;
 	return (ISC_R_SUCCESS);
 
- failure:
-	dns_zonemgr_destroy(&zmgr);
+ free_taskpool:
+	isc_taskpool_destroy(&zmgr->zonetasks);	
+ free_conflock:
+	isc_rwlock_destroy(&zmgr->conflock);
+ free_rwlock:
+	isc_rwlock_destroy(&zmgr->rwlock);
+ free_mem:
+	isc_mem_put(zmgr->mctx, zmgr, sizeof *zmgr);
 	return (result);
 }
 
@@ -3093,7 +3113,18 @@ dns_zonemgr_destroy(dns_zonemgr_t **zmgrp) {
 	/* Probably done already, but does not hurt to repeat. */
 	dns_zonemgr_shutdown(zmgr);
 
+	isc_rwlock_destroy(&zmgr->conflock);
 	isc_rwlock_destroy(&zmgr->rwlock);
 	isc_mem_put(zmgr->mctx, zmgr, sizeof *zmgr);
 	*zmgrp = NULL;
 }
+
+void
+dns_zonemgr_lockconf(dns_zonemgr_t *zmgr, isc_rwlocktype_t type) {
+	RWLOCK(&zmgr->conflock, type);
+}
+
+void
+dns_zonemgr_unlockconf(dns_zonemgr_t *zmgr, isc_rwlocktype_t type) {
+	RWUNLOCK(&zmgr->conflock, type);
+}