validators can now be cancelled.

lib/dns/include/dns/validator.h

@@ -15,7 +15,7 @@
  * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: validator.h,v 1.12 2000/08/01 01:24:54 tale Exp $ */
+/* $Id: validator.h,v 1.13 2000/08/15 00:21:05 bwelling Exp $ */
 
 #ifndef DNS_VALIDATOR_H
 #define DNS_VALIDATOR_H 1
@@ -70,6 +70,42 @@ typedef struct dns_validatorevent {
 	dns_message_t *			message;
 } dns_validatorevent_t;
 
+
+/*
+ * A validator object represents a validation in procgress.
+ *
+ * Clients are strongly discouraged from using this type directly, with
+ * the exception of the 'link' field, which may be used directly for
+ * whatever purpose the client desires.
+ */
+struct dns_validator {
+	/* Unlocked. */
+	unsigned int			magic;
+	isc_mutex_t			lock;
+	dns_view_t *			view;
+	/* Locked by lock. */
+	unsigned int			options;
+	unsigned int			attributes;
+	dns_validatorevent_t *		event;
+	dns_fetch_t *			fetch;
+	dns_validator_t *		keyvalidator;
+	dns_validator_t *		authvalidator;
+	dns_keytable_t *		keytable;
+	dns_keynode_t *			keynode;
+	dst_key_t *			key;
+	dns_rdata_sig_t *		siginfo;
+	isc_task_t *			task;
+	isc_taskaction_t		action;
+	void *				arg;
+	unsigned int			labels;
+	dns_rdataset_t *		currentset;
+	isc_boolean_t			seensig;
+	dns_rdataset_t *		keyset;
+	dns_rdataset_t			frdataset;
+	dns_rdataset_t			fsigrdataset;
+	ISC_LINK(dns_validator_t)	link;
+};
+
 ISC_LANG_BEGINDECLS
 
 isc_result_t

lib/dns/resolver.c

@@ -15,7 +15,7 @@
  * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: resolver.c,v 1.163 2000/08/09 19:11:27 gson Exp $ */
+/* $Id: resolver.c,v 1.164 2000/08/15 00:21:03 bwelling Exp $ */
 
 #include <config.h>
 
@@ -170,11 +170,11 @@ struct fetchctx {
 	dns_adbaddrinfolist_t		forwaddrs;
 	isc_sockaddrlist_t		forwarders;
 	isc_sockaddrlist_t		bad;
+	ISC_LIST(dns_validator_t)	validators;
 	/*
 	 * # of events we're waiting for.
 	 */
 	unsigned int			pending;
-	unsigned int			validating;
 	unsigned int			restarts;
 };
 
@@ -1155,7 +1155,7 @@ fctx_finddone(isc_task_t *task, isc_event_t *event) {
 			want_done = ISC_TRUE;
 		}
 	} else if (SHUTTINGDOWN(fctx) && fctx->pending == 0 &&
-		   fctx->validating == 0) {
+		   ISC_LIST_EMPTY(fctx->validators)) {
 		bucketnum = fctx->bucketnum;
 		LOCK(&res->buckets[bucketnum].lock);
 		/*
@@ -1680,7 +1680,7 @@ fctx_destroy(fetchctx_t *fctx) {
 	REQUIRE(ISC_LIST_EMPTY(fctx->queries));
 	REQUIRE(ISC_LIST_EMPTY(fctx->finds));
 	REQUIRE(fctx->pending == 0);
-	REQUIRE(fctx->validating == 0);
+	REQUIRE(ISC_LIST_EMPTY(fctx->validators));
 	REQUIRE(fctx->references == 0);
 
 	FCTXTRACE("destroy");
@@ -1792,6 +1792,7 @@ fctx_doshutdown(isc_task_t *task, isc_event_t *event) {
 	isc_boolean_t bucket_empty = ISC_FALSE;
 	dns_resolver_t *res;
 	unsigned int bucketnum;
+	dns_validator_t *validator;
 
 	REQUIRE(VALID_FCTX(fctx));
 
@@ -1809,6 +1810,16 @@ fctx_doshutdown(isc_task_t *task, isc_event_t *event) {
 	 */
 	fctx->attributes &= ~FCTX_ATTR_ADDRWAIT;
 
+	/*
+	 * Cancel all pending validators.  Note that this must be done
+	 * without the bucket lock held, since that could cause deadlock.
+	 */
+	validator = ISC_LIST_HEAD(fctx->validators);
+	while (validator != NULL) {
+		dns_validator_cancel(validator);
+		validator = ISC_LIST_NEXT(validator, link);
+	}
+
 	LOCK(&res->buckets[bucketnum].lock);
 
 	INSIST(fctx->state == fetchstate_active ||
@@ -1822,7 +1833,7 @@ fctx_doshutdown(isc_task_t *task, isc_event_t *event) {
 	}
 
 	if (fctx->references == 0 && fctx->pending == 0 &&
-	    fctx->validating == 0)
+	    ISC_LIST_EMPTY(fctx->validators))
 		bucket_empty = fctx_destroy(fctx);
 
 	UNLOCK(&res->buckets[bucketnum].lock);
@@ -1863,7 +1874,7 @@ fctx_start(isc_task_t *task, isc_event_t *event) {
 		 * pending ADB finds and no pending validations.
 		 */
 		INSIST(fctx->pending == 0);
-		INSIST(fctx->validating == 0);
+		INSIST(ISC_LIST_EMPTY(fctx->validators));
 		if (fctx->references == 0) {
 			/*
 			 * It's now safe to destroy this fctx.
@@ -2037,9 +2048,9 @@ fctx_create(dns_resolver_t *res, dns_name_t *name, dns_rdatatype_t type,
 	ISC_LIST_INIT(fctx->forwaddrs);
 	ISC_LIST_INIT(fctx->forwarders);
 	ISC_LIST_INIT(fctx->bad);
+	ISC_LIST_INIT(fctx->validators);
 	fctx->find = NULL;
 	fctx->pending = 0;
-	fctx->validating = 0;
 	fctx->restarts = 0;
 	if (dns_name_requiresedns(name))
 		fctx->attributes = FCTX_ATTR_NEEDEDNS0;
@@ -2230,7 +2241,7 @@ maybe_destroy(fetchctx_t *fctx) {
 
 	REQUIRE(SHUTTINGDOWN(fctx));
 
-	if (fctx->pending != 0 || fctx->validating != 0)
+	if (fctx->pending != 0 || !ISC_LIST_EMPTY(fctx->validators))
 		return;
 
 	bucketnum = fctx->bucketnum;
@@ -2265,20 +2276,20 @@ validated(isc_task_t *task, isc_event_t *event) {
 	REQUIRE(event->ev_type == DNS_EVENT_VALIDATORDONE);
 	fctx = event->ev_arg;
 	REQUIRE(VALID_FCTX(fctx));
-	REQUIRE(fctx->validating > 0);
+	REQUIRE(!ISC_LIST_EMPTY(fctx->validators));
 
 	vevent = (dns_validatorevent_t *)event;
 
 	FCTXTRACE("received validation completion event");
 
+	ISC_LIST_UNLINK(fctx->validators, vevent->validator, link);
+
 	/*
 	 * Destroy the validator early so that we can
 	 * destroy the fctx if necessary.
 	 */
 	dns_validator_destroy(&vevent->validator);
 
-	fctx->validating--;
-
 	negative = ISC_TF(vevent->rdataset == NULL);
 
 	sentresponse = ISC_TF((fctx->options & DNS_FETCHOPT_NOVALIDATE) != 0);
@@ -2404,7 +2415,7 @@ validated(isc_task_t *task, isc_event_t *event) {
 		goto cleanup_event;
 	}
 
-	if (fctx->validating > 0) {
+	if (!ISC_LIST_EMPTY(fctx->validators)) {
 		INSIST(!negative);
 		INSIST(fctx->type == dns_rdatatype_any ||
 		       fctx->type == dns_rdatatype_sig);
@@ -2647,7 +2658,9 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, isc_stdtime_t now) {
 						fctx,
 						&validator);
 					if (result == ISC_R_SUCCESS)
-						fctx->validating++;
+						ISC_LIST_APPEND(
+							fctx->validators,
+							validator, link);
 				}
 			}
 		} else if (!EXTERNAL(rdataset)) {
@@ -2726,7 +2739,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, isc_stdtime_t now) {
 					      fctx,
 					      &validator);
 		if (result == ISC_R_SUCCESS)
-			fctx->validating++;
+			ISC_LIST_APPEND(fctx->validators, validator, link);
 	}
 
 	if (result == ISC_R_SUCCESS && have_answer) {
@@ -2909,7 +2922,7 @@ ncache_message(fetchctx_t *fctx, dns_rdatatype_t covers, isc_stdtime_t now) {
 					      &validator);
 		if (result != ISC_R_SUCCESS)
 			return (result);
-		fctx->validating++;
+		ISC_LIST_APPEND(fctx->validators, validator, link);
 		/*
 		 * If validation is necessary, return now.  Otherwise continue
 		 * to process the message, letting the validation complete
@@ -4872,7 +4885,7 @@ dns_resolver_destroyfetch(dns_fetch_t **fetchp) {
 		/*
 		 * No one cares about the result of this fetch anymore.
 		 */
-		if (fctx->pending == 0 && fctx->validating == 0 &&
+		if (fctx->pending == 0 && ISC_LIST_EMPTY(fctx->validators) &&
 		    SHUTTINGDOWN(fctx)) {
 			/*
 			 * This fctx is already shutdown; we were just

lib/dns/validator.c

@@ -15,7 +15,7 @@
  * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: validator.c,v 1.72 2000/08/14 22:17:40 gson Exp $ */
+/* $Id: validator.c,v 1.73 2000/08/15 00:21:02 bwelling Exp $ */
 
 #include <config.h>
 
@@ -40,33 +40,6 @@
 #include <dns/validator.h>
 #include <dns/view.h>
 
-struct dns_validator {
-	/* Unlocked. */
-	unsigned int			magic;
-	isc_mutex_t			lock;
-	dns_view_t *			view;
-	/* Locked by lock. */
-	unsigned int			options;
-	unsigned int			attributes;
-	dns_validatorevent_t *		event;
-	dns_fetch_t *			fetch;
-	dns_validator_t *		keyvalidator;
-	dns_validator_t *		authvalidator;
-	dns_keytable_t *		keytable;
-	dns_keynode_t *			keynode;
-	dst_key_t *			key;
-	dns_rdata_sig_t *		siginfo;
-	isc_task_t *			task;
-	isc_taskaction_t		action;
-	void *				arg;
-	unsigned int			labels;
-	dns_rdataset_t *		currentset;
-	isc_boolean_t			seensig;
-	dns_rdataset_t *		keyset;
-	dns_rdataset_t			frdataset;
-	dns_rdataset_t			fsigrdataset;
-};
-
 #define VALIDATOR_MAGIC			0x56616c3fU	/* Val?. */
 #define VALID_VALIDATOR(v)	 	ISC_MAGIC_VALID(v, VALIDATOR_MAGIC)
 
@@ -101,7 +74,8 @@ static void
 validator_done(dns_validator_t *val, isc_result_t result) {
 	isc_task_t *task;
 
-	REQUIRE(val->event != NULL);
+	if (val->event == NULL)
+		return;
 
 	/*
 	 * Caller must be holding the lock.
@@ -157,6 +131,17 @@ fetch_callback_validator(isc_task_t *task, isc_event_t *event) {
 	isc_event_free(&event);
 	dns_resolver_destroyfetch(&val->fetch);
 
+	if (SHUTDOWN(val)) {
+		dns_validator_destroy(&val);
+		return;
+	}
+
+	if (val->event == NULL) {
+		validator_log(val, ISC_LOG_DEBUG(3),
+			      "fetch_callback_validator: event == NULL");
+		return;
+	}
+
 	validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_validator");
 	LOCK(&val->lock);
 	if (eresult == ISC_R_SUCCESS) {
@@ -199,7 +184,6 @@ fetch_callback_nullkey(isc_task_t *task, isc_event_t *event) {
 	dns_fetchevent_t *devent;
 	dns_validator_t *val;
 	dns_rdataset_t *rdataset, *sigrdataset;
-	dns_fetch_t *fetch;
 	isc_result_t result;
 	isc_result_t eresult;
 
@@ -211,9 +195,23 @@ fetch_callback_nullkey(isc_task_t *task, isc_event_t *event) {
 	sigrdataset = &val->fsigrdataset;
 	eresult = devent->result;
 
+	dns_resolver_destroyfetch(&val->fetch);
+
+	if (SHUTDOWN(val)) {
+		dns_validator_destroy(&val);
+		isc_event_free(&event);
+		return;
+	}
+
+	if (val->event == NULL) {
+		validator_log(val, ISC_LOG_DEBUG(3),
+			      "fetch_callback_nullkey: event == NULL");
+		isc_event_free(&event);
+		return;
+	}
+
 	validator_log(val, ISC_LOG_DEBUG(3), "in fetch_callback_nullkey");
-	fetch = val->fetch;
+
-	val->fetch = NULL;
 	LOCK(&val->lock);
 	if (eresult == ISC_R_SUCCESS) {
 		if (!containsnullkey(val, rdataset)) {
@@ -251,10 +249,9 @@ fetch_callback_nullkey(isc_task_t *task, isc_event_t *event) {
 				if (result != ISC_R_SUCCESS)
 					validator_done(val, result);
 				/*
-				 * Don't free these, since they'll be
+				 * Don't free rdataset & sigrdataset, since
-				 * freed in nullkeyvalidated.
+				 * they'll be freed in nullkeyvalidated.
 				 */
-				dns_resolver_destroyfetch(&fetch);
 				isc_event_free(&event);
 				UNLOCK(&val->lock);
 				return;
@@ -281,8 +278,6 @@ fetch_callback_nullkey(isc_task_t *task, isc_event_t *event) {
 	}
 	UNLOCK(&val->lock);
 
-	dns_resolver_destroyfetch(&fetch);
-
 	/*
 	 * Free stuff from the event.
 	 */
@@ -302,6 +297,15 @@ keyvalidated(isc_task_t *task, isc_event_t *event) {
 
 	UNUSED(task);
 	INSIST(event->ev_type == DNS_EVENT_VALIDATORDONE);
+
+	if (SHUTDOWN(val)) {
+		dns_validator_destroy(&val);
+		return;
+	}
+
+	if (val->event == NULL)
+		return;
+
 	devent = (dns_validatorevent_t *)event;
 	val = devent->ev_arg;
 	eresult = devent->result;
@@ -427,6 +431,15 @@ authvalidated(isc_task_t *task, isc_event_t *event) {
 
 	UNUSED(task);
 	INSIST(event->ev_type == DNS_EVENT_VALIDATORDONE);
+
+	if (SHUTDOWN(val)) {
+		dns_validator_destroy(&val);
+		return;
+	}
+
+	if (val->event == NULL)
+		return;
+
 	devent = (dns_validatorevent_t *)event;
 	rdataset = devent->rdataset;
 	sigrdataset = devent->sigrdataset;
@@ -470,6 +483,15 @@ negauthvalidated(isc_task_t *task, isc_event_t *event) {
 
 	UNUSED(task);
 	INSIST(event->ev_type == DNS_EVENT_VALIDATORDONE);
+
+	if (SHUTDOWN(val)) {
+		dns_validator_destroy(&val);
+		return;
+	}
+
+	if (val->event == NULL)
+		return;
+
 	devent = (dns_validatorevent_t *)event;
 	val = devent->ev_arg;
 	eresult = devent->result;
@@ -509,6 +531,15 @@ nullkeyvalidated(isc_task_t *task, isc_event_t *event) {
 
 	UNUSED(task);
 	INSIST(event->ev_type == DNS_EVENT_VALIDATORDONE);
+
+	if (SHUTDOWN(val)) {
+		dns_validator_destroy(&val);
+		return;
+	}
+
+	if (val->event == NULL)
+		return;
+
 	devent = (dns_validatorevent_t *)event;
 	val = devent->ev_arg;
 	eresult = devent->result;
@@ -1285,6 +1316,10 @@ validator_start(isc_task_t *task, isc_event_t *event) {
 	vevent = (dns_validatorevent_t *)event;
 	val = vevent->validator;
 
+	/* If the validator has been cancelled, val->event == NULL */
+	if (val->event == NULL)
+		return;
+
 	validator_log(val, ISC_LOG_DEBUG(3), "starting");
 
 	LOCK(&val->lock);
@@ -1408,6 +1443,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
 	val->seensig = ISC_FALSE;
 	dns_rdataset_init(&val->frdataset);
 	dns_rdataset_init(&val->fsigrdataset);
+	ISC_LINK_INIT(val, link);
 	val->magic = VALIDATOR_MAGIC;
 
 	isc_task_send(task, (isc_event_t **)&event);
@@ -1435,18 +1471,19 @@ dns_validator_cancel(dns_validator_t *validator) {
 
 	LOCK(&validator->lock);
 
+	validator_log(validator, ISC_LOG_DEBUG(3), "dns_validator_cancel");
+
 	if (validator->event != NULL) {
-		validator->event->result = ISC_R_CANCELED;
+		validator_done(validator, ISC_R_CANCELED);
-		task = validator->event->ev_sender;
-		validator->event->ev_sender = validator;
-		isc_task_sendanddetach(&task,
-				       (isc_event_t **)&validator->event);
 
 		if (validator->fetch != NULL)
 			dns_resolver_cancelfetch(validator->fetch);
 
 		if (validator->keyvalidator != NULL)
 			dns_validator_cancel(validator->keyvalidator);
+
+		if (validator->authvalidator != NULL)
+			dns_validator_cancel(validator->authvalidator);
 	}
 	UNLOCK(&validator->lock);
 }
@@ -1458,9 +1495,6 @@ destroy(dns_validator_t *val) {
 	REQUIRE(SHUTDOWN(val));
 	REQUIRE(val->event == NULL);
 	REQUIRE(val->fetch == NULL);
-#if 0
-	REQUIRE(val->currentset == NULL);
-#endif
 
 	if (val->keynode != NULL)
 		dns_keytable_detachkeynode(val->keytable, &val->keynode);
@@ -1492,8 +1526,11 @@ dns_validator_destroy(dns_validator_t **validatorp) {
 
 	REQUIRE(val->event == NULL);
 
+	validator_log(val, ISC_LOG_DEBUG(3), "dns_validator_destroy");
+
 	val->attributes |= VALATTR_SHUTDOWN;
-	if (val->fetch == NULL)
+	if (val->fetch == NULL && val->keyvalidator == NULL &&
+	    val->authvalidator == NULL)
 		want_destroy = ISC_TRUE;
 
 	UNLOCK(&val->lock);