refactor plugin hook resumption to use loop callbacks

plugins supporting asynchronous operation now use a loop callback
to resume operation in query_hookresume() rather than a task.

bin/tests/system/hooks/driver/test-async.c

@@ -19,6 +19,7 @@
 #include <stdbool.h>
 #include <string.h>
 
+#include <isc/async.h>
 #include <isc/buffer.h>
 #include <isc/hash.h>
 #include <isc/ht.h>
@@ -30,7 +31,6 @@
 #include <isc/util.h>
 
 #include <ns/client.h>
-#include <ns/events.h>
 #include <ns/hooks.h>
 #include <ns/log.h>
 #include <ns/query.h>
@@ -59,7 +59,7 @@ typedef struct async_instance {
 
 typedef struct state {
 	bool async;
-	ns_hook_resevent_t *rev;
+	ns_hook_resume_t *rev;
 	ns_hookpoint_t hookpoint;
 	isc_result_t origresult;
 } state_t;
@@ -277,29 +277,31 @@ destroyasync(ns_hookasync_t **ctxp) {
 }
 
 static isc_result_t
-doasync(query_ctx_t *qctx, isc_mem_t *mctx, void *arg, isc_task_t *task,
-	isc_taskaction_t action, void *evarg, ns_hookasync_t **ctxp) {
-	ns_hook_resevent_t *rev = (ns_hook_resevent_t *)isc_event_allocate(
-		mctx, task, NS_EVENT_HOOKASYNCDONE, action, evarg,
-		sizeof(*rev));
+doasync(query_ctx_t *qctx, isc_mem_t *mctx, void *arg, isc_loop_t *loop,
+	isc_job_cb cb, void *evarg, ns_hookasync_t **ctxp) {
+	ns_hook_resume_t *rev = isc_mem_get(mctx, sizeof(*rev));
 	ns_hookasync_t *ctx = isc_mem_get(mctx, sizeof(*ctx));
 	state_t *state = (state_t *)arg;
 
 	logmsg("doasync");
-	*ctx = (ns_hookasync_t){ .mctx = NULL };
+	*ctx = (ns_hookasync_t){
+		.cancel = cancelasync,
+		.destroy = destroyasync,
+	};
 	isc_mem_attach(mctx, &ctx->mctx);
-	ctx->cancel = cancelasync;
-	ctx->destroy = destroyasync;
 
-	rev->hookpoint = state->hookpoint;
-	rev->origresult = state->origresult;
 	qctx->result = DNS_R_NOTIMP;
-	rev->saved_qctx = qctx;
-	rev->ctx = ctx;
+	*rev = (ns_hook_resume_t){
+		.hookpoint = state->hookpoint,
+		.origresult = qctx->result,
+		.saved_qctx = qctx,
+		.ctx = ctx,
+		.arg = evarg,
+	};
 
 	state->rev = rev;
 
-	isc_task_send(task, (isc_event_t **)&rev);
+	isc_async_run(loop, cb, rev);
 
 	*ctxp = ctx;
 	return (ISC_R_SUCCESS);

lib/dns/client.c

@@ -115,7 +115,7 @@ typedef struct resctx {
 	dns_fetch_t *fetch;
 	dns_namelist_t namelist;
 	isc_result_t result;
-	dns_clientresevent_t *event;
+	dns_clientresume_t *event;
 	dns_rdataset_t *rdataset;
 	dns_rdataset_t *sigrdataset;
 } resctx_t;
@@ -878,7 +878,7 @@ client_resfind(resctx_t *rctx, dns_fetchevent_t *event) {
 static void
 resolve_done(isc_task_t *task, isc_event_t *event) {
 	resarg_t *resarg = event->ev_arg;
-	dns_clientresevent_t *rev = (dns_clientresevent_t *)event;
+	dns_clientresume_t *rev = (dns_clientresume_t *)event;
 	dns_name_t *name = NULL;
 	isc_result_t result;
 
@@ -955,7 +955,7 @@ dns_client_startresolve(dns_client_t *client, const dns_name_t *name,
 			unsigned int options, isc_task_t *task,
 			isc_taskaction_t action, void *arg,
 			dns_clientrestrans_t **transp) {
-	dns_clientresevent_t *event = NULL;
+	dns_clientresume_t *event = NULL;
 	resctx_t *rctx = NULL;
 	isc_task_t *tclone = NULL;
 	isc_mem_t *mctx;
@@ -980,7 +980,7 @@ dns_client_startresolve(dns_client_t *client, const dns_name_t *name,
 	 */
 	tclone = NULL;
 	isc_task_attach(task, &tclone);
-	event = (dns_clientresevent_t *)isc_event_allocate(
+	event = (dns_clientresume_t *)isc_event_allocate(
 		mctx, tclone, DNS_EVENT_CLIENTRESDONE, action, arg,
 		sizeof(*event));
 	event->result = DNS_R_SERVFAIL;

lib/dns/include/dns/client.h

@@ -73,7 +73,7 @@ ISC_LANG_BEGINDECLS
 #define DNS_CLIENTVIEW_NAME "_dnsclient"
 
 /*%
- * A dns_clientresevent_t is sent when name resolution performed by a client
+ * A dns_clientresume_t is sent when name resolution performed by a client
  * completes.  'result' stores the result code of the entire resolution
  * procedure.  'vresult' specifically stores the result code of DNSSEC
  * validation if it is performed.  When name resolution successfully completes,
@@ -81,12 +81,12 @@ ISC_LANG_BEGINDECLS
  * RRsets.  It is the receiver's responsibility to free this list by calling
  * dns_client_freeresanswer() before freeing the event structure.
  */
-typedef struct dns_clientresevent {
-	ISC_EVENT_COMMON(struct dns_clientresevent);
+typedef struct dns_clientresume {
+	ISC_EVENT_COMMON(struct dns_clientresume);
 	isc_result_t   result;
 	isc_result_t   vresult;
 	dns_namelist_t answerlist;
-} dns_clientresevent_t; /* too long? */
+} dns_clientresume_t; /* too long? */
 
 isc_result_t
 dns_client_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr,
@@ -232,7 +232,7 @@ dns_client_startresolve(dns_client_t *client, const dns_name_t *name,
  *
  * dns_client_startresolve() is an asynchronous version of dns_client_resolve()
  * and does not block.  When name resolution is completed, 'action' will be
- * called with the argument of a 'dns_clientresevent_t' object, which contains
+ * called with the argument of a 'dns_clientresume_t' object, which contains
  * the resulting list of answer names (on success).  On return, '*transp' is
  * set to an opaque transaction ID so that the caller can cancel this
  * resolution process.

lib/dns/include/dns/resolver.h

@@ -52,6 +52,7 @@
 #include <isc/lang.h>
 #include <isc/refcount.h>
 #include <isc/stats.h>
+#include <isc/tls.h>
 #include <isc/types.h>
 
 #include <dns/fixedname.h>

lib/ns/include/ns/hooks.h

@@ -22,7 +22,6 @@
 #include <isc/magic.h>
 #include <isc/mem.h>
 #include <isc/result.h>
-#include <isc/task.h>
 
 #include <dns/rdatatype.h>
 
@@ -198,15 +197,15 @@
  * asynchronous event by calling ns_query_hookasync().  This is similar
  * to ns_query_recurse(), but more generic.  ns_query_hookasync() will
  * call the 'runasync' function with a specified 'arg' (both passed to
- * ns_query_hookasync()) and a set of task and associated event arguments
+ * ns_query_hookasync()) and a set of loop and associated event arguments
  * to be called to resume query handling upon completion of the
  * asynchronous event.
  *
  * The implementation of 'runasync' is assumed to allocate and build an
- * instance of ns_hook_resevent_t whose action, arg, and task are set to
+ * instance of ns_hook_resume_t whose callback, arg, and loop are set to
  * the passed values from ns_query_hookasync().  Other fields of
- * ns_hook_resevent_t must be correctly set in the hook implementation
- * by the time it's sent to the specified task:
+ * ns_hook_resume_t must be correctly set in the hook implementation
+ * by the time it's sent to the specified loop:
  *
  * - hookpoint: the point from which the query handling should be resumed
  *   (which should usually be the hook point that triggered the asynchronous
@@ -229,7 +228,7 @@
  * NS_HOOK_RETURN to suspend the query handling.
  *
  * On the completion of the asynchronous event, the hook implementation is
- * supposed to send the resumeevent to the corresponding task.  The query
+ * supposed to send the resumeevent to the corresponding loop.  The query
  * module resumes the query handling so that the hook action of the
  * specified hook point will be called, skipping some intermediate query
  * handling steps.  So, typically, the same hook action will be called
@@ -242,7 +241,7 @@
  *
  * typedef struct hookstate {
  * 	bool async;
- *	ns_hook_resevent_t *rev
+ *	ns_hook_resume_t *rev
  *	ns_hookpoint_t hookpoint;
  *	isc_result_t origresult;
  * } hookstate_t;
@@ -276,28 +275,30 @@
  * And the 'runasync' function would be something like this:
  *
  * static isc_result_t
- * runasync(query_ctx_t *qctx, void *arg, isc_taskaction_t action,
- *	    void *evarg, isc_task_t *task, ns_hookasync_t **ctxp) {
+ * runasync(query_ctx_t *qctx, void *arg, isc_job_cb cb, void *evarg,
+ * 	    isc_loop_t *loop, ns_hookasync_t **ctxp) {
  * 	hookstate_t *state = arg;
- *	ns_hook_resevent_t *rev = isc_event_allocate(
- *		mctx, task, NS_EVENT_HOOKASYNCDONE, action, evarg,
- *		sizeof(*rev));
+ *	ns_hook_resume_t *rev isc_mem_get(mctx, sizeof(*rev));
  *	ns_hookasync_t *ctx = isc_mem_get(mctx, sizeof(*ctx));
  *
- *	*ctx = (ns_hookasync_t){ .private = NULL };
+ *	*ctx = (ns_hookasync_t){ .private = NULL
+ *                               .hookpoint = state->hookpoint,
+ *                               .origresult = state->origresult,
+ *                               .saved_ctx = qctx,
+ *                               .ctx = ctx,
+ *                               .loop = loop,
+ *                               .cb = cb,
+ *                               .arg = cbarg
+ *	};
  *	isc_mem_attach(mctx, &ctx->mctx);
+ *
  *	ctx->cancel = ...;  // set the cancel function, which cancels the
  *			    // internal asynchronous event (if necessary).
  *			    // it should eventually result in sending
- *			    // the 'rev' event to the calling task.
+ *			    // the 'rev' event to the calling loop.
  *	ctx->destroy = ...; // set the destroy function, which frees 'ctx'
  *
- *	rev->hookpoint = state->hookpoint;
- *	rev->origresult = state->origresult;
- *	rev->saved_qctx = qctx;
- *	rev->ctx = ctx;
- *
- *	state->rev = rev; // store the resume event so we can send it later
+ *	state->rev = rev; // store the resume state so we can send it later
  *
  *	// initiate some asynchronous process here - for example, a
  *	// recursive fetch.
@@ -312,8 +313,7 @@
  *
  * static void
  * asyncproc_done(hookstate_t *state) {
- *	isc_event_t *ev = (isc_event_t *)state->rev;
- *	isc_task_send(ev->ev_sender, &ev);
+ *	isc_async_run(state->rev->loop, state->rev->cb, state->rev->arg);
  * }
  *
  * Caveats:
@@ -438,13 +438,15 @@ struct ns_hookasync {
  * isc_event to be sent on the completion of a hook-initiated asyncronous
  * process, similar to dns_fetchevent_t.
  */
-typedef struct ns_hook_resevent {
-	ISC_EVENT_COMMON(struct ns_hook_resevent);
+typedef struct ns_hook_resume {
 	ns_hookasync_t *ctx;	   /* asynchronous processing context */
 	ns_hookpoint_t	hookpoint; /* hook point from which to resume */
 	isc_result_t origresult; /* result code at the point of call to hook */
 	query_ctx_t *saved_qctx; /* qctx at the point of call to hook */
-} ns_hook_resevent_t;
+	isc_loop_t  *loop;	 /* loopmgr loop to resume in */
+	isc_job_cb   cb;	 /* callback function */
+	void	    *arg;	 /* argument to pass to the callback */
+} ns_hook_resume_t;
 
 /*
  * Plugin API version
@@ -455,7 +457,7 @@ typedef struct ns_hook_resevent {
  * as well; if not, set NS_PLUGIN_AGE to 0.
  */
 #ifndef NS_PLUGIN_VERSION
-#define NS_PLUGIN_VERSION 1
+#define NS_PLUGIN_VERSION 2
 #define NS_PLUGIN_AGE	  0
 #endif /* ifndef NS_PLUGIN_VERSION */
 

lib/ns/include/ns/query.h

@@ -19,7 +19,6 @@
 
 #include <isc/buffer.h>
 #include <isc/netaddr.h>
-#include <isc/task.h>
 #include <isc/types.h>
 
 #include <dns/rdataset.h>
@@ -243,9 +242,11 @@ struct query_ctx {
 	int	     line;   /* line to report error */
 };
 
-typedef isc_result_t (*ns_query_starthookasync_t)(
-	query_ctx_t *qctx, isc_mem_t *mctx, void *arg, isc_task_t *task,
-	isc_taskaction_t action, void *evarg, ns_hookasync_t **ctxp);
+typedef isc_result_t (*ns_query_starthookasync_t)(query_ctx_t *qctx,
+						  isc_mem_t *mctx, void *arg,
+						  isc_loop_t *loop,
+						  isc_job_cb cb, void *evarg,
+						  ns_hookasync_t **ctxp);
 
 /*
  * The following functions are expected to be used only within query.c

lib/ns/query.c

@@ -6761,18 +6761,16 @@ cleanup:
 }
 
 static void
-query_hookresume(isc_task_t *task, isc_event_t *event) {
-	ns_hook_resevent_t *rev = (ns_hook_resevent_t *)event;
+query_hookresume(void *arg) {
+	ns_hook_resume_t *rev = (ns_hook_resume_t *)arg;
 	ns_hookasync_t *hctx = NULL;
-	ns_client_t *client = rev->ev_arg;
+	ns_client_t *client = rev->arg;
 	query_ctx_t *qctx = rev->saved_qctx;
 	bool canceled;
 
 	CTRACE(ISC_LOG_DEBUG(3), "query_hookresume");
 
 	REQUIRE(NS_CLIENT_VALID(client));
-	REQUIRE(task == client->manager->task);
-	REQUIRE(event->ev_type == NS_EVENT_HOOKASYNCDONE);
 
 	LOCK(&client->query.fetchlock);
 	if (client->query.hookactx != NULL) {
@@ -6898,10 +6896,10 @@ query_hookresume(isc_task_t *task, isc_event_t *event) {
 		}
 	}
 
+	isc_mem_put(hctx->mctx, rev, sizeof(*rev));
 	hctx->destroy(&hctx);
 	qctx_destroy(qctx);
 	isc_mem_put(client->manager->mctx, qctx, sizeof(*qctx));
-	isc_event_free(&event);
 }
 
 isc_result_t
@@ -6925,7 +6923,7 @@ ns_query_hookasync(query_ctx_t *qctx, ns_query_starthookasync_t runasync,
 	saved_qctx = isc_mem_get(client->manager->mctx, sizeof(*saved_qctx));
 	qctx_save(qctx, saved_qctx);
 	result = runasync(saved_qctx, client->manager->mctx, arg,
-			  client->manager->task, query_hookresume, client,
+			  client->manager->loop, query_hookresume, client,
 			  &client->query.hookactx);
 	if (result != ISC_R_SUCCESS) {
 		goto cleanup_and_detach_from_quota;

tests/ns/query_test.c

@@ -611,7 +611,7 @@ typedef struct hookasync_data {
 				       * asynchronous process */
 	bool canceled;		      /* true if the query has been canceled  */
 	isc_result_t start_result;    /* result of 'runasync' */
-	ns_hook_resevent_t *rev;      /* resume event sent on completion */
+	ns_hook_resume_t *rev;	      /* resume state sent on completion */
 	query_ctx_t qctx;	      /* shallow copy of qctx passed to hook */
 	ns_hookpoint_t hookpoint;     /* specifies where to resume */
 	ns_hookpoint_t lasthookpoint; /* remember the last hook point called */
@@ -640,31 +640,36 @@ cancel_hookactx(ns_hookasync_t *ctx) {
 /* 'runasync' callback passed to ns_query_hookasync */
 static isc_result_t
 test_hookasync(query_ctx_t *qctx, isc_mem_t *memctx, void *arg,
-	       isc_task_t *task, isc_taskaction_t action, void *evarg,
+	       isc_loop_t *loop, isc_job_cb cb, void *evarg,
 	       ns_hookasync_t **ctxp) {
 	hookasync_data_t *asdata = arg;
 	ns_hookasync_t *ctx = NULL;
-	ns_hook_resevent_t *rev = NULL;
+	ns_hook_resume_t *rev = NULL;
 
 	if (asdata->start_result != ISC_R_SUCCESS) {
 		return (asdata->start_result);
 	}
 
 	ctx = isc_mem_get(memctx, sizeof(*ctx));
-	rev = (ns_hook_resevent_t *)isc_event_allocate(
-		memctx, task, NS_EVENT_HOOKASYNCDONE, action, evarg,
-		sizeof(*rev));
+	rev = isc_mem_get(memctx, sizeof(*rev));
+	*rev = (ns_hook_resume_t){
+		.hookpoint = asdata->hookpoint,
+		.origresult = DNS_R_NXDOMAIN,
+		.saved_qctx = qctx,
+		.ctx = ctx,
+		.loop = loop,
+		.cb = cb,
+		.arg = evarg,
+	};
 
-	rev->hookpoint = asdata->hookpoint;
-	rev->origresult = DNS_R_NXDOMAIN;
-	rev->saved_qctx = qctx;
-	rev->ctx = ctx;
 	asdata->rev = rev;
 
-	*ctx = (ns_hookasync_t){ .private = asdata };
+	*ctx = (ns_hookasync_t){
+		.destroy = destroy_hookactx,
+		.cancel = cancel_hookactx,
+		.private = asdata,
+	};
 	isc_mem_attach(memctx, &ctx->mctx);
-	ctx->destroy = destroy_hookactx;
-	ctx->cancel = cancel_hookactx;
 
 	*ctxp = ctx;
 	return (ISC_R_SUCCESS);
@@ -948,8 +953,7 @@ run_hookasync_test(const ns__query_hookasync_test_params_t *test) {
 	/* If async event has started, manually invoke the 'done' event. */
 	if (asdata.async) {
 		qctx->client->now = 0; /* set to sentinel before resume */
-		asdata.rev->ev_action(asdata.rev->ev_sender,
-				      (isc_event_t *)asdata.rev);
+		asdata.rev->cb(asdata.rev);
 
 		/* Confirm necessary cleanup has been performed. */
 		INSIST(qctx->client->query.hookactx == NULL);
@@ -1266,7 +1270,7 @@ typedef struct {
 typedef struct hookasync_e2e_data {
 	bool async;		   /* true if in a hook-triggered
 				    * asynchronous process */
-	ns_hook_resevent_t *rev;   /* resume event sent on completion */
+	ns_hook_resume_t *rev;	   /* resume state sent on completion */
 	ns_hookpoint_t hookpoint;  /* specifies where to resume */
 	isc_result_t start_result; /* result of 'runasync' */
 	dns_rcode_t expected_rcode;
@@ -1282,10 +1286,10 @@ cancel_e2ehookactx(ns_hookasync_t *ctx) {
 /* 'runasync' callback passed to ns_query_hookasync */
 static isc_result_t
 test_hookasync_e2e(query_ctx_t *qctx, isc_mem_t *memctx, void *arg,
-		   isc_task_t *task, isc_taskaction_t action, void *evarg,
+		   isc_loop_t *loop, isc_job_cb cb, void *evarg,
 		   ns_hookasync_t **ctxp) {
 	ns_hookasync_t *ctx = NULL;
-	ns_hook_resevent_t *rev = NULL;
+	ns_hook_resume_t *rev = NULL;
 	hookasync_e2e_data_t *asdata = arg;
 
 	if (asdata->start_result != ISC_R_SUCCESS) {
@@ -1293,19 +1297,24 @@ test_hookasync_e2e(query_ctx_t *qctx, isc_mem_t *memctx, void *arg,
 	}
 
 	ctx = isc_mem_get(memctx, sizeof(*ctx));
-	rev = (ns_hook_resevent_t *)isc_event_allocate(
-		memctx, task, NS_EVENT_HOOKASYNCDONE, action, evarg,
-		sizeof(*rev));
+	rev = isc_mem_get(memctx, sizeof(*rev));
+	*rev = (ns_hook_resume_t){
+		.hookpoint = asdata->hookpoint,
+		.saved_qctx = qctx,
+		.ctx = ctx,
+		.loop = loop,
+		.cb = cb,
+		.arg = evarg,
+	};
 
-	rev->hookpoint = asdata->hookpoint;
-	rev->saved_qctx = qctx;
-	rev->ctx = ctx;
 	asdata->rev = rev;
 
-	*ctx = (ns_hookasync_t){ .private = asdata };
+	*ctx = (ns_hookasync_t){
+		.destroy = destroy_hookactx,
+		.cancel = cancel_e2ehookactx,
+		.private = asdata,
+	};
 	isc_mem_attach(memctx, &ctx->mctx);
-	ctx->destroy = destroy_hookactx;
-	ctx->cancel = cancel_e2ehookactx;
 
 	*ctxp = ctx;
 	return (ISC_R_SUCCESS);
@@ -1413,8 +1422,7 @@ run_hookasync_e2e_test(const ns__query_hookasync_e2e_test_params_t *test) {
 		 * If async event has started, manually invoke the done event.
 		 */
 		INSIST(asdata.async);
-		asdata.rev->ev_action(asdata.rev->ev_sender,
-				      (isc_event_t *)asdata.rev);
+		asdata.rev->cb(asdata.rev);
 
 		/*
 		 * Usually 'async' is reset to false on the 2nd call to