From b65cbc7dc3e08e96f9f1980214153932c252ae1e Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Sep 2019 11:59:34 +0930 Subject: [PATCH] plugins: fix false-positive memleak. This moves field initialization into plugins_new(), and adds a memleak helper to search the request map: =================================== ERRORS ==================================== ___________________ ERROR at teardown of test_plugin_command ___________________ [gw0] linux -- Python 3.7.1 /opt/python/3.7.1/bin/python3.7 > lambda: ihook(item=item, **kwds), when=when, ) ../../../.local/lib/python3.7/site-packages/flaky/flaky_pytest_plugin.py:306: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ tests/fixtures.py:112: in node_factory ok = nf.killall([not n.may_fail for n in nf.nodes]) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = , expected_successes = [True] def killall(self, expected_successes): """Returns true if every node we expected to succeed actually succeeded"" unexpected_fail = False for i in range(len(self.nodes)): leaks = None # leak detection upsets VALGRIND by reading uninitialized mem. # If it's dead, we'll catch it below. if not VALGRIND: try: # This also puts leaks in log. leaks = self.nodes[i].rpc.dev_memleak()['leaks'] except Exception: pass try: self.nodes[i].stop() except Exception: if expected_successes[i]: unexpected_fail = True if leaks is not None and len(leaks) != 0: raise Exception("Node {} has memory leaks: {}".format( self.nodes[i].daemon.lightning_dir, > json.dumps(leaks, sort_keys=True, indent=4) )) E Exception: Node /tmp/ltests-qm87my20/test_plugin_command_1/lightnng-1/ has memory leaks: [ E { E "backtrace": [ E "ccan/ccan/tal/tal.c:437 (tal_alloc_)", E "lightningd/jsonrpc.c:1112 (jsonrpc_request_start_)", E "lightningd/plugin.c:1041 (plugin_config)", E "lightningd/plugin.c:1072 (plugins_config)", E "lightningd/plugin.c:846 (plugin_manifest_cb)", E "lightningd/plugin.c:252 (plugin_response_handle)", E "lightningd/plugin.c:342 (plugin_read_json_one)", E "lightningd/plugin.c:367 (plugin_read_json)", E "ccan/ccan/io/io.c:59 (next_plan)", E "ccan/ccan/io/io.c:407 (do_plan)", E "ccan/ccan/io/io.c:417 (io_ready)", E "ccan/ccan/io/poll.c:445 (io_loop)", E "lightningd/io_loop_with_timers.c:24 (io_loop_with_tiers)", E "lightningd/lightningd.c:840 (main)" E ], E "label": "lightningd/jsonrpc.c:1112:struct jsonrpc_reques", E "parents": [ E "lightningd/plugin.c:66:struct plugin", E "lightningd/lightningd.c:103:struct lightningd" E ], E "value": "0x55d6385e4088" E }, E { E "backtrace": [ E "ccan/ccan/tal/tal.c:437 (tal_alloc_)", E "lightningd/jsonrpc.c:1112 (jsonrpc_request_start_)", E "lightningd/plugin.c:1041 (plugin_config)", E "lightningd/plugin.c:1072 (plugins_config)", E "lightningd/plugin.c:846 (plugin_manifest_cb)", E "lightningd/plugin.c:252 (plugin_response_handle)", E "lightningd/plugin.c:342 (plugin_read_json_one)", E "lightningd/plugin.c:367 (plugin_read_json)", E "ccan/ccan/io/io.c:59 (next_plan)", E "ccan/ccan/io/io.c:407 (do_plan)", E "ccan/ccan/io/io.c:417 (io_ready)", E "ccan/ccan/io/poll.c:445 (io_loop)", E "lightningd/io_loop_with_timers.c:24 (io_loop_with_tiers)", E "lightningd/lightningd.c:840 (main)" E ], E "label": "lightningd/jsonrpc.c:1112:struct jsonrpc_reques", E "parents": [ E "lightningd/plugin.c:66:struct plugin", E "lightningd/lightningd.c:103:struct lightningd" E ], E "value": "0x55d6386529d8" E } E ] Signed-off-by: Rusty Russell --- lightningd/plugin.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 69d4e824d..036fc57a8 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -33,6 +33,14 @@ * `getmanifest` call anyway, that's what `init `is for. */ #define PLUGIN_MANIFEST_TIMEOUT 60 +#if DEVELOPER +static void memleak_help_pending_requests(struct htable *memtable, + struct plugins *plugins) +{ + memleak_remove_uintmap(memtable, &plugins->pending_requests); +} +#endif /* DEVELOPER */ + struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, struct lightningd *ld) { @@ -42,6 +50,9 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, p->log_book = log_book; p->log = new_log(p, log_book, "plugin-manager"); p->ld = ld; + uintmap_init(&p->pending_requests); + memleak_add_helper(p, memleak_help_pending_requests); + return p; } @@ -1007,8 +1018,6 @@ void plugins_start(struct plugins *plugins, const char *dev_plugin_debug) void plugins_init(struct plugins *plugins, const char *dev_plugin_debug) { plugins->pending_manifests = 0; - uintmap_init(&plugins->pending_requests); - plugins_add_default_dir(plugins, path_join(tmpctx, plugins->ld->config_dir, "plugins"));