plugins: allow 'before' and 'after' arrays for hooks.

The next patch will use these to order the hooks.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: plugins: hooks can now specify that they must be called 'before' or 'after' other plugins.

doc/PLUGINS.md

@@ -93,8 +93,8 @@ example:
     "disconnect"
   ],
   "hooks": [
-    "openchannel",
-    "htlc_accepted"
+    { "name": "openchannel", "before": ["another_plugin"] },
+    { "name": "htlc_accepted" }
   ],
   "features": {
     "node": "D0000000",
@@ -702,13 +702,20 @@ declares that it'd like to be consulted on what to do next for certain
 events in the daemon. A hook can then decide how `lightningd` should
 react to the given event.
 
+When hooks are registered, they can optionally specify "before" and
+"after" arrays of plugin names, which control what order they will be
+called in.  If a plugin name is unknown, it is ignored, otherwise if the
+hook calls cannot be ordered to satisfy the specifications of all
+plugin hooks, the plugin registration will fail.
+
 The call semantics of the hooks, i.e., when and how hooks are called, depend
 on the hook type. Most hooks are currently set to `single`-mode. In this mode
 only a single plugin can register the hook, and that plugin will get called
 for each event of that type. If a second plugin attempts to register the hook
 it gets killed and a corresponding log entry will be added to the logs. In
 `chain`-mode multiple plugins can register for the hook type and they are
-called sequentially if a matching event is triggered. Each plugin can then
+called in any order which meets their `before` and `after` requirements
+if a matching event is triggered. Each plugin can then
 handle the event or defer by returning a `continue` result like the following:
 
 ```json

lightningd/plugin.c

@@ -117,7 +117,6 @@ static void destroy_plugin(struct plugin *p)
 {
 	struct plugin_rpccall *call;
 
-	plugin_hook_unregister_all(p);
 	list_del(&p->list);
 
 	/* Terminate all pending RPC calls with an error. */
@@ -1057,20 +1056,43 @@ static const char *plugin_subscriptions_add(struct plugin *plugin,
 static const char *plugin_hooks_add(struct plugin *plugin, const char *buffer,
 				    const jsmntok_t *resulttok)
 {
-	const jsmntok_t *hookstok = json_get_member(buffer, resulttok, "hooks");
+	const jsmntok_t *t, *hookstok, *beforetok, *aftertok;
+	size_t i;
+
+	hookstok = json_get_member(buffer, resulttok, "hooks");
 	if (!hookstok)
 		return NULL;
 
-	for (int i = 0; i < hookstok->size; i++) {
-		char *name = json_strdup(tmpctx, plugin->buffer,
-					 json_get_arr(hookstok, i));
-		if (!plugin_hook_register(plugin, name)) {
+	json_for_each_arr(i, t, hookstok) {
+		char *name;
+		struct plugin_hook *hook;
+
+		if (t->type == JSMN_OBJECT) {
+			const jsmntok_t *nametok;
+
+			nametok = json_get_member(buffer, t, "name");
+			if (!nametok)
+				return tal_fmt(plugin, "no name in hook obj %.*s",
+					       json_tok_full_len(t),
+					       json_tok_full(buffer, t));
+			name = json_strdup(tmpctx, buffer, nametok);
+			beforetok = json_get_member(buffer, t, "before");
+			aftertok = json_get_member(buffer, t, "after");
+		} else {
+			name = json_strdup(tmpctx, plugin->buffer, t);
+			beforetok = aftertok = NULL;
+		}
+
+		hook = plugin_hook_register(plugin, name);
+		if (!hook) {
 			return tal_fmt(plugin,
 				    "could not register hook '%s', either the "
 				    "name doesn't exist or another plugin "
 				    "already registered it.",
 				    name);
 		}
+
+		plugin_hook_add_deps(hook, plugin, buffer, beforetok, aftertok);
 		tal_free(name);
 	}
 	return NULL;

lightningd/plugin_hook.c

@@ -18,6 +18,14 @@ struct plugin_hook_request {
 	struct lightningd *ld;
 };
 
+struct hook_instance {
+	/* What plugin registered */
+	struct plugin *plugin;
+
+	/* Dependencies it asked for. */
+	const char **before, **after;
+};
+
 /* A link in the plugin_hook call chain (there's a joke in there about
  * computer scientists and naming...). The purpose is to act both as a list
  * from which elements can be popped off as we progress along the chain as
@@ -43,61 +51,52 @@ static struct plugin_hook *plugin_hook_by_name(const char *name)
 	return NULL;
 }
 
-bool plugin_hook_register(struct plugin *plugin, const char *method)
+/* When we destroy a plugin, we remove its hooks */
+static void destroy_hook_instance(struct hook_instance *h,
+				  struct plugin_hook *hook)
 {
+	for (size_t i = 0; i < tal_count(hook->hooks); i++) {
+		if (h == hook->hooks[i]) {
+			tal_arr_remove(&hook->hooks, i);
+			return;
+		}
+	}
+	abort();
+}
+
+struct plugin_hook *plugin_hook_register(struct plugin *plugin, const char *method)
+{
+	struct hook_instance *h;
 	struct plugin_hook *hook = plugin_hook_by_name(method);
 	if (!hook) {
 		/* No such hook name registered */
-		return false;
+		return NULL;
 	}
 
-	/* Make sure the plugins array is initialized. */
-	if (hook->plugins == NULL)
-		hook->plugins = notleak(tal_arr(NULL, struct plugin *, 0));
+	/* Make sure the hook_elements array is initialized. */
+	if (hook->hooks == NULL)
+		hook->hooks = notleak(tal_arr(NULL, struct hook_instance *, 0));
 
 	/* If this is a single type hook and we have a plugin registered we
 	 * must fail this attempt to add the plugin to the hook. */
-	if (hook->type == PLUGIN_HOOK_SINGLE && tal_count(hook->plugins) > 0)
-		return false;
+	if (hook->type == PLUGIN_HOOK_SINGLE && tal_count(hook->hooks) > 0)
+		return NULL;
 
 	/* Ensure we don't register the same plugin multple times. */
-	for (size_t i=0; i<tal_count(hook->plugins); i++)
-		if (hook->plugins[i] == plugin)
-			return true;
+	for (size_t i=0; i<tal_count(hook->hooks); i++)
+		if (hook->hooks[i]->plugin == plugin)
+			return NULL;
 
 	/* Ok, we're sure they can register and they aren't yet registered, so
 	 * register them. */
-	tal_arr_expand(&hook->plugins, plugin);
-	return true;
-}
+	h = tal(plugin, struct hook_instance);
+	h->plugin = plugin;
+	h->before = tal_arr(h, const char *, 0);
+	h->after = tal_arr(h, const char *, 0);
+	tal_add_destructor2(h, destroy_hook_instance, hook);
 
-bool plugin_hook_unregister(struct plugin *plugin, const char *method)
-{
-	struct plugin_hook *hook = plugin_hook_by_name(method);
-
-	if (!hook || !hook->plugins) {
-		/* No such hook name registered */
-		return false;
-	}
-
-	for (size_t i = 0; i < tal_count(hook->plugins); i++) {
-		if (hook->plugins[i] == plugin) {
-			tal_arr_remove(&hook->plugins, i);
-			return true;
-		}
-	}
-	return false;
-}
-
-void plugin_hook_unregister_all(struct plugin *plugin)
-{
-	static struct plugin_hook **hooks = NULL;
-	static size_t num_hooks;
-	if (!hooks)
-		hooks = autodata_get(hooks, &num_hooks);
-
-	for (size_t i = 0; i < num_hooks; i++)
-		plugin_hook_unregister(plugin, hooks[i]->name);
+	tal_arr_expand(&hook->hooks, h);
+	return hook;
 }
 
 /* Mutual recursion */
@@ -244,23 +243,24 @@ bool plugin_hook_call_(struct lightningd *ld, const struct plugin_hook *hook,
 {
 	struct plugin_hook_request *ph_req;
 	struct plugin_hook_call_link *link;
-	if (tal_count(hook->plugins)) {
+	if (tal_count(hook->hooks)) {
 		/* If we have a plugin that has registered for this
 		 * hook, serialize and call it */
 		/* FIXME: technically this is a leak, but we don't
 		 * currently have a list to store these. We might want
 		 * to eventually to inspect in-flight requests. */
-		ph_req = notleak(tal(hook->plugins, struct plugin_hook_request));
+		ph_req = notleak(tal(hook->hooks, struct plugin_hook_request));
 		ph_req->hook = hook;
 		ph_req->cb_arg = tal_steal(ph_req, cb_arg);
 		ph_req->db = ld->wallet->db;
 		ph_req->ld = ld;
 
 		list_head_init(&ph_req->call_chain);
-		for (size_t i=0; i<tal_count(hook->plugins); i++) {
+		for (size_t i=0; i<tal_count(hook->hooks); i++) {
 			/* We allocate this off of the plugin so we get notified if the plugin dies. */
-			link = tal(hook->plugins[i], struct plugin_hook_call_link);
-			link->plugin = hook->plugins[i];
+			link = tal(hook->hooks[i]->plugin,
+				   struct plugin_hook_call_link);
+			link->plugin = hook->hooks[i]->plugin;
 			link->req = ph_req;
 			tal_add_destructor(link, plugin_hook_killed);
 			list_add_tail(&ph_req->call_chain, &link->list);
@@ -324,10 +324,10 @@ void plugin_hook_db_sync(struct db *db)
 	struct plugin *plugin;
 
 	const char **changes = db_changes(db);
-	if (tal_count(hook->plugins) == 0)
+	if (tal_count(hook->hooks) == 0)
 		return;
 
-	ph_req = notleak(tal(hook->plugins, struct plugin_hook_request));
+	ph_req = notleak(tal(hook->hooks, struct plugin_hook_request));
 	/* FIXME: do IO logging for this! */
 	req = jsonrpc_request_start(NULL, hook->name, NULL, NULL,
 				    db_hook_response,
@@ -335,7 +335,7 @@ void plugin_hook_db_sync(struct db *db)
 
 	ph_req->hook = hook;
 	ph_req->db = db;
-	plugin = ph_req->plugin = hook->plugins[0];
+	plugin = ph_req->plugin = hook->hooks[0]->plugin;
 
 	json_add_num(req->stream, "data_version", db_data_version_get(db));
 
@@ -357,3 +357,38 @@ void plugin_hook_db_sync(struct db *db)
 		io_break(ret);
 	}
 }
+
+static void add_deps(const char ***arr,
+		     const char *buffer,
+		     const jsmntok_t *arrtok)
+{
+	const jsmntok_t *t;
+	size_t i;
+
+	if (!arrtok)
+		return;
+
+	json_for_each_arr(i, t, arrtok)
+		tal_arr_expand(arr, json_strdup(*arr, buffer, t));
+}
+
+void plugin_hook_add_deps(struct plugin_hook *hook,
+			  struct plugin *plugin,
+			  const char *buffer,
+			  const jsmntok_t *before,
+			  const jsmntok_t *after)
+{
+	struct hook_instance *h = NULL;
+
+	/* We just added this, it must exist */
+	for (size_t i = 0; i < tal_count(hook->hooks); i++) {
+		if (hook->hooks[i]->plugin == plugin) {
+			h = hook->hooks[i];
+			break;
+		}
+	}
+	assert(h);
+
+	add_deps(&h->before, buffer, before);
+	add_deps(&h->after, buffer, after);
+}

lightningd/plugin_hook.h

@@ -77,7 +77,7 @@ struct plugin_hook {
 
 	/* Which plugins have registered this hook? This is a `tal_arr`
 	 * initialized at creation. */
-	struct plugin **plugins;
+	struct hook_instance **hooks;
 };
 AUTODATA_TYPE(hooks, struct plugin_hook);
 
@@ -155,15 +155,17 @@ bool plugin_hook_continue(void *arg, const char *buffer, const jsmntok_t *toks);
 	AUTODATA(hooks, &name##_hook_gen);                                     \
 	PLUGIN_HOOK_CALL_DEF(name, cb_arg_type)
 
-bool plugin_hook_register(struct plugin *plugin, const char *method);
-
-/* Unregister a hook a plugin has registered for */
-bool plugin_hook_unregister(struct plugin *plugin, const char *method);
-
-/* Unregister all hooks a plugin has registered for */
-void plugin_hook_unregister_all(struct plugin *plugin);
+struct plugin_hook *plugin_hook_register(struct plugin *plugin,
+					 const char *method);
 
 /* Special sync plugin hook for db. */
 void plugin_hook_db_sync(struct db *db);
 
+/* Add dependencies for this hook. */
+void plugin_hook_add_deps(struct plugin_hook *hook,
+			  struct plugin *plugin,
+			  const char *buffer,
+			  const jsmntok_t *before,
+			  const jsmntok_t *after);
+
 #endif /* LIGHTNING_LIGHTNINGD_PLUGIN_HOOK_H */