From 10338983a6f09e62ffff18a79d754b99ad54439d Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Thu, 1 Nov 2018 18:53:49 +0100 Subject: [PATCH] plugin: Add logs to plugin and add method to kill a plugin Also includes some sanity checks for the results returned by the plugin, such as ensuring that the ID is as expected and that we have either an error or a real result. --- lightningd/lightningd.c | 2 +- lightningd/plugin.c | 32 ++++++++++++++++++++++----- lightningd/plugin.h | 3 ++- lightningd/test/run-find_my_abspath.c | 2 +- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 18f357dc6..b646b0d7a 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -217,7 +217,7 @@ static struct lightningd *new_lightningd(const tal_t *ctx) *code. Here we initialize the context that will keep track and control *the plugins. */ - ld->plugins = plugins_new(ld); + ld->plugins = plugins_new(ld, ld->log); return ld; } diff --git a/lightningd/plugin.c b/lightningd/plugin.c index d5df9072f..d009132b4 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -23,6 +23,8 @@ struct plugin { /* Stuff we write */ struct list_head output; const char *outbuf; + + struct log *log; }; struct plugin_request { @@ -46,6 +48,7 @@ struct plugins { /* Currently pending requests by their request ID */ UINTMAP(struct plugin_request *) pending_requests; + struct log *log; }; struct json_output { @@ -53,10 +56,11 @@ struct json_output { const char *json; }; -struct plugins *plugins_new(const tal_t *ctx){ +struct plugins *plugins_new(const tal_t *ctx, struct log *log){ struct plugins *p; p = tal(ctx, struct plugins); p->plugins = tal_arr(p, struct plugin *, 0); + p->log = log; return p; } @@ -69,6 +73,18 @@ void plugin_register(struct plugins *plugins, const char* path TAKES) plugins->plugins[n] = p; p->plugins = plugins; p->cmd = tal_strdup(p, path); + p->log = plugins->log; +} + +/** + * Kill a plugin process, with an error message. + */ +static void plugin_kill(struct plugin *plugin, char *msg) +{ + log_broken(plugin->log, "Killing plugin: %s", msg); + plugin->stop = true; + io_wake(plugin); + kill(plugin->pid, SIGKILL); } /** @@ -91,8 +107,8 @@ static bool plugin_read_json_one(struct plugin *plugin) toks = json_parse_input(plugin->buffer, plugin->used, &valid); if (!toks) { if (!valid) { - /* FIXME (cdecker) Print error and kill the plugin */ - return io_close(plugin->stdout_conn); + plugin_kill(plugin, "Failed to parse JSON response"); + return false; } /* We need more. */ return false; @@ -108,23 +124,24 @@ static bool plugin_read_json_one(struct plugin *plugin) errortok = json_get_member(plugin->buffer, toks, "error"); idtok = json_get_member(plugin->buffer, toks, "id"); - /* FIXME(cdecker) Kill the plugin if either of these fails */ if (!idtok) { + plugin_kill(plugin, "JSON-RPC response does not contain an \"id\"-field"); return false; } else if (!resulttok && !errortok) { + plugin_kill(plugin, "JSON-RPC response does not contain a \"result\" or \"error\" field"); return false; } /* We only send u64 ids, so if this fails it's a critical error */ if (!json_to_u64(plugin->buffer, idtok, &id)) { - /* FIXME (cdecker) Log an error message and kill the plugin */ + plugin_kill(plugin, "JSON-RPC response \"id\"-field is not a u64"); return false; } request = uintmap_get(&plugin->plugins->pending_requests, id); if (!request) { - /* FIXME(cdecker) Log an error and kill the plugin */ + plugin_kill(plugin, "Received a JSON-RPC response for non-existent request"); return false; } @@ -155,6 +172,9 @@ static struct io_plan *plugin_read_json(struct io_conn *conn UNUSED, success = plugin_read_json_one(plugin); } while (success); + if (plugin->stop) + return io_close(plugin->stdout_conn); + /* Now read more from the connection */ return io_read_partial(plugin->stdout_conn, plugin->buffer + plugin->used, diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 57d79d158..1b3f1df9c 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -4,6 +4,7 @@ #include #include #include +#include /** * A collection of plugins, and some associated information. @@ -15,7 +16,7 @@ struct plugins; /** * Create a new plugins context. */ -struct plugins *plugins_new(const tal_t *ctx); +struct plugins *plugins_new(const tal_t *ctx, struct log *log); /** * Initialize the registered plugins. diff --git a/lightningd/test/run-find_my_abspath.c b/lightningd/test/run-find_my_abspath.c index 5169c06e4..38df6b72d 100644 --- a/lightningd/test/run-find_my_abspath.c +++ b/lightningd/test/run-find_my_abspath.c @@ -126,7 +126,7 @@ void onchaind_replay_channels(struct lightningd *ld UNNEEDED) void plugins_init(struct plugins *plugins UNNEEDED) { fprintf(stderr, "plugins_init called!\n"); abort(); } /* Generated stub for plugins_new */ -struct plugins *plugins_new(const tal_t *ctx UNNEEDED) +struct plugins *plugins_new(const tal_t *ctx UNNEEDED, struct log *log UNNEEDED) { fprintf(stderr, "plugins_new called!\n"); abort(); } /* Generated stub for register_opts */ void register_opts(struct lightningd *ld UNNEEDED)