From 5b58eda7486d2fe613396e197d9a538f84a85820 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 16 Sep 2022 12:45:03 +0930 Subject: [PATCH] libplugin: mark the cmd notleak() whenever command_still_pending() called. This is what we do in lightningd, which makes memleak much more forgiving: you can hang temporaries off cmd without getting reports of leaks (also when send_outreq called). We remove all the notleak() calls in plugins which worked around this! And avoid multiple notleak labels, since both send_outreq() and command_still_pending() can be called multiple times. Signed-off-by: Rusty Russell --- common/memleak.c | 16 ++++++++++------ plugins/bcli.c | 3 --- plugins/bkpr/bookkeeper.c | 8 +------- plugins/commando.c | 11 ++--------- plugins/libplugin.c | 4 ++++ plugins/pay.c | 4 ---- plugins/spender/fundchannel.c | 3 --- plugins/spender/multifundchannel.c | 7 ++----- 8 files changed, 19 insertions(+), 37 deletions(-) diff --git a/common/memleak.c b/common/memleak.c index a12b32001..205e3804e 100644 --- a/common/memleak.c +++ b/common/memleak.c @@ -50,12 +50,16 @@ void *notleak_(void *ptr, bool plus_children) name = tal_name(ptr); if (!name) name = ""; - if (plus_children) - name = tal_fmt(tmpctx, "%s **NOTLEAK_IGNORE_CHILDREN**", - name); - else - name = tal_fmt(tmpctx, "%s **NOTLEAK**", name); - tal_set_name(ptr, name); + + /* Don't mark more than once! */ + if (!strstr(name, "**NOTLEAK")) { + if (plus_children) + name = tal_fmt(tmpctx, "%s **NOTLEAK_IGNORE_CHILDREN**", + name); + else + name = tal_fmt(tmpctx, "%s **NOTLEAK**", name); + tal_set_name(ptr, name); + } return cast_const(void *, ptr); } diff --git a/plugins/bcli.c b/plugins/bcli.c index b5c6a9b80..47bc121f5 100644 --- a/plugins/bcli.c +++ b/plugins/bcli.c @@ -782,9 +782,6 @@ static struct command_result *sendrawtransaction(struct command *cmd, } else highfeesarg = NULL; - /* Keep memleak happy! */ - tal_free(allowhighfees); - start_bitcoin_cli(NULL, cmd, process_sendrawtransaction, true, BITCOIND_HIGH_PRIO, NULL, "sendrawtransaction", diff --git a/plugins/bkpr/bookkeeper.c b/plugins/bkpr/bookkeeper.c index bece4a959..be0b256ef 100644 --- a/plugins/bkpr/bookkeeper.c +++ b/plugins/bkpr/bookkeeper.c @@ -1294,7 +1294,7 @@ static struct command_result *lookup_invoice_desc(struct command *cmd, struct out_req *req; /* Otherwise will go away when event is cleaned up */ - notleak(tal_steal(cmd, payment_hash)); + tal_steal(cmd, payment_hash); if (!amount_msat_zero(credit)) req = jsonrpc_request_start(cmd->plugin, cmd, "listinvoices", @@ -1372,8 +1372,6 @@ listpeers_done(struct command *cmd, const char *buf, if (info->ev->payment_id && streq(info->ev->tag, mvt_tag_str(INVOICE))) { - /* Make memleak happy */ - tal_steal(tmpctx, info); return lookup_invoice_desc(cmd, info->ev->credit, info->ev->payment_id); } @@ -1591,8 +1589,6 @@ parse_and_log_chain_move(struct command *cmd, if (tags[i] != INVOICE) continue; - /* Keep memleak happy */ - tal_steal(tmpctx, e); return lookup_invoice_desc(cmd, e->credit, e->payment_id); } @@ -1673,8 +1669,6 @@ parse_and_log_channel_move(struct command *cmd, maybe_record_rebalance(db, e); db_commit_transaction(db); - /* Keep memleak happy */ - tal_steal(tmpctx, e); return lookup_invoice_desc(cmd, e->credit, e->payment_id); } diff --git a/plugins/commando.c b/plugins/commando.c index ae770e376..1f33ffaa6 100644 --- a/plugins/commando.c +++ b/plugins/commando.c @@ -680,13 +680,12 @@ static struct command_result *json_commando(struct command *cmd, tal_append_fmt(&json, ",\"rune\":\"%s\"", rune); tal_append_fmt(&json, "}"); - /* This is not a leak, but we don't keep a pointer. */ - outgoing = notleak(tal(cmd, struct outgoing)); + outgoing = tal(cmd, struct outgoing); outgoing->peer = *peer; outgoing->msg_off = 0; /* 65000 per message gives sufficient headroom. */ jsonlen = tal_bytelen(json)-1; - outgoing->msgs = notleak(tal_arr(cmd, u8 *, (jsonlen + 64999) / 65000)); + outgoing->msgs = tal_arr(cmd, u8 *, (jsonlen + 64999) / 65000); for (size_t i = 0; i < tal_count(outgoing->msgs); i++) { u8 *cmd_msg = tal_arr(outgoing, u8, 0); bool terminal = (i == tal_count(outgoing->msgs) - 1); @@ -705,12 +704,6 @@ static struct command_result *json_commando(struct command *cmd, outgoing->msgs[i] = cmd_msg; } - /* Keep memleak code happy! */ - tal_free(peer); - tal_free(method); - tal_free(cparams); - tal_free(rune); - return send_more_cmd(cmd, NULL, NULL, outgoing); } diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 5b2701b81..ad2e59fbc 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -297,6 +297,8 @@ struct command_result *command_finished(struct command *cmd, struct command_result *WARN_UNUSED_RESULT command_still_pending(struct command *cmd) { + if (cmd) + notleak_with_children(cmd); return &pending; } @@ -728,6 +730,8 @@ send_outreq(struct plugin *plugin, const struct out_req *req) ld_rpc_send(plugin, req->js); + if (req->cmd != NULL) + notleak_with_children(req->cmd); return &pending; } diff --git a/plugins/pay.c b/plugins/pay.c index c67e48bd7..2f99e702c 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -1202,10 +1202,6 @@ static struct command_result *json_pay(struct command *cmd, } payment_mod_exemptfee_get_data(p)->amount = exemptfee ? *exemptfee : AMOUNT_MSAT(5000); - - /* We free unneeded params now to keep memleak happy. */ - tal_free(maxfee_pct_millionths); - tal_free(exemptfee); } shadow_route = payment_mod_shadowroute_get_data(p); diff --git a/plugins/spender/fundchannel.c b/plugins/spender/fundchannel.c index 51332c6c1..ba5adadab 100644 --- a/plugins/spender/fundchannel.c +++ b/plugins/spender/fundchannel.c @@ -108,9 +108,6 @@ json_fundchannel(struct command *cmd, if (utxos) json_add_tok(req->js, "utxos", utxos, buf); - /* Stop memleak from complaining */ - tal_free(id); - return send_outreq(cmd->plugin, req); } diff --git a/plugins/spender/multifundchannel.c b/plugins/spender/multifundchannel.c index ffb50b053..c639efdc0 100644 --- a/plugins/spender/multifundchannel.c +++ b/plugins/spender/multifundchannel.c @@ -1832,8 +1832,8 @@ post_cleanup_redo_multifundchannel(struct multifundchannel_redo *redo) /* Okay, we still have destinations to try: wait a second in case it * takes that long to disconnect from peer, then retry. */ - notleak(plugin_timer(mfc->cmd->plugin, time_from_sec(1), - perform_multifundchannel, mfc)); + plugin_timer(mfc->cmd->plugin, time_from_sec(1), + perform_multifundchannel, mfc); return command_still_pending(mfc->cmd); } @@ -2064,9 +2064,6 @@ json_multifundchannel(struct command *cmd, mfc->sigs_collected = false; - /* Stop memleak from complaining */ - tal_free(minconf); - perform_multifundchannel(mfc); return command_still_pending(mfc->cmd); }