aljaz/lightning
1
0
Fork 0
mirror of https://github.com/aljazceru/lightning.git synced 2026-01-06 07:34:21 +01:00
Code Issues Packages Projects Releases Wiki Activity

plugin: don't call notification after free.

Browse Source 
This is an old bug, where a plugin can get called while we're shutting
down (and have freed plugins), but it's triggered more reliably by the
new warning notification hook.

For good measure, we also make freeing a plugin self-delete.

Valgrind error file: valgrind-errors.16763
==16886== Invalid read of size 8
==16886==    at 0x422919: plugins_notify (plugin.c:1096)
==16886==    by 0x413919: notify_warning (notification.c:61)
==16886==    by 0x412BDE: logv (log.c:251)
==16886==    by 0x412A98: log_ (log.c:311)
==16886==    by 0x4044BE: bcli_finished (bitcoind.c:178)
==16886==    by 0x459480: destroy_conn (poll.c:244)
==16886==    by 0x459499: destroy_conn_close_fd (poll.c:250)
==16886==    by 0x4619E1: notify (tal.c:235)
==16886==    by 0x461A7E: del_tree (tal.c:397)
==16886==    by 0x461AB5: del_tree (tal.c:407)
==16886==    by 0x461AB5: del_tree (tal.c:407)
==16886==    by 0x461AB5: del_tree (tal.c:407)
==16886==  Address 0x634a578 is 40 bytes inside a block of size 352 free'd
==16886==    at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16886==    by 0x461AFD: del_tree (tal.c:416)
==16886==    by 0x461FB7: tal_free (tal.c:481)
==16886==    by 0x411E0A: main (lightningd.c:841)
==16886==  Block was alloc'd at
==16886==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16886==    by 0x4617CE: allocate (tal.c:245)
==16886==    by 0x461E4C: tal_alloc_ (tal.c:423)
==16886==    by 0x42255E: plugins_new (plugin.c:106)
==16886==    by 0x41133D: new_lightningd (lightningd.c:218)
==16886==    by 0x411AD4: main (lightningd.c:649)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell
2019-06-12 10:08:55 +09:30
parent c6ec9443b9
commit 5a520f4a07
2 changed files with 16 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
lightningd/lightningd.c
View File

@@ -838,7 +838,8 @@ int main(int argc, char *argv[])
shutdown_subdaemons(ld);
tal_free(ld->plugins);
/* Remove plugins. */
ld->plugins = tal_free(ld->plugins);
/* Clean up the JSON-RPC. This needs to happen in a DB transaction since
* it might actually be touching the DB in some destructors, e.g.,

17
lightningd/plugin.c
View File

@@ -112,6 +112,11 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book,
return p;
}
static void destroy_plugin(struct plugin *p)
{
list_del(&p->list);
}
void plugin_register(struct plugins *plugins, const char* path TAKES)
{
struct plugin *p;
@@ -126,6 +131,7 @@ void plugin_register(struct plugins *plugins, const char* path TAKES)
path_basename(tmpctx, p->cmd));
p->methods = tal_arr(p, const char *, 0);
list_head_init(&p->plugin_opts);
tal_add_destructor(p, destroy_plugin);
}
static bool paths_match(const char *cmd, const char *name)
@@ -1093,9 +1099,14 @@ void plugins_notify(struct plugins *plugins,
const struct jsonrpc_notification *n TAKES)
{
struct plugin *p;
list_for_each(&plugins->plugins, p, list) {
if (plugin_subscriptions_contains(p, n->method))
plugin_send(p, json_stream_dup(p, n->stream, p->log));
/* If we're shutting down, ld->plugins will be NULL */
if (plugins) {
list_for_each(&plugins->plugins, p, list) {
if (plugin_subscriptions_contains(p, n->method))
plugin_send(p, json_stream_dup(p, n->stream,
p->log));
}
}
if (taken(n))
tal_free(n);