From e2a31f42f2a8da21c018d9ea65c1a620650b3b34 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 30 Oct 2020 11:43:37 +1030 Subject: [PATCH] plugins: allow 'before' and 'after' arrays for hooks. The next patch will use these to order the hooks. Signed-off-by: Rusty Russell Changelog-Added: plugins: hooks can now specify that they must be called 'before' or 'after' other plugins. --- doc/PLUGINS.md | 13 +++- lightningd/plugin.c | 34 ++++++++-- lightningd/plugin_hook.c | 131 +++++++++++++++++++++++++-------------- lightningd/plugin_hook.h | 18 +++--- 4 files changed, 131 insertions(+), 65 deletions(-) diff --git a/doc/PLUGINS.md b/doc/PLUGINS.md index d41a7ddb4..8ea5d3288 100644 --- a/doc/PLUGINS.md +++ b/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 diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 679b59d56..d9ff8b737 100644 --- a/lightningd/plugin.c +++ b/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; diff --git a/lightningd/plugin_hook.c b/lightningd/plugin_hook.c index 351d9f887..c5e83914a 100644 --- a/lightningd/plugin_hook.c +++ b/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; iplugins); i++) - if (hook->plugins[i] == plugin) - return true; + for (size_t i=0; ihooks); 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; iplugins); i++) { + for (size_t i=0; ihooks); 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); +} diff --git a/lightningd/plugin_hook.h b/lightningd/plugin_hook.h index 272659336..2cf17a6f9 100644 --- a/lightningd/plugin_hook.h +++ b/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 */