Skip to content

Commit

Permalink
closingd: handle our own memleak detection.
Browse files Browse the repository at this point in the history
Unlike other daemons, closingd doesn't listen to the master, but runs
simply to its own beat.  So instead of responding to the JSON dev_memleak
command, we always check for memory leaks, and make sure that the
python tests fail if they see MEMLEAK in the logs.

Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
rustyrussell committed Nov 22, 2018
1 parent 55306fc commit 8fb1b60
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 1 deletion.
30 changes: 30 additions & 0 deletions closingd/closingd.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <common/crypto_sync.h>
#include <common/derive_basepoints.h>
#include <common/htlc.h>
#include <common/memleak.h>
#include <common/peer_billboard.h>
#include <common/peer_failed.h>
#include <common/read_peer_msg.h>
Expand Down Expand Up @@ -426,6 +427,27 @@ static u64 adjust_offer(struct crypto_state *cs,
return (feerange->max + min_fee_to_accept)/2;
}

#if DEVELOPER
/* FIXME: We should talk to lightningd anyway, rather than doing this */
static void closing_dev_memleak(const tal_t *ctx,
u8 *scriptpubkey[NUM_SIDES],
const u8 *funding_wscript)
{
struct htable *memtable;

memtable = memleak_enter_allocations(tmpctx,
scriptpubkey[LOCAL],
scriptpubkey[REMOTE]);

/* Now delete known pointers (these aren't really roots, just
* pointers we know are referenced).*/
memleak_remove_referenced(memtable, ctx);
memleak_remove_referenced(memtable, funding_wscript);

dump_memleak(memtable);
}
#endif /* DEVELOPER */

int main(int argc, char *argv[])
{
setup_locale();
Expand Down Expand Up @@ -490,6 +512,9 @@ int main(int argc, char *argv[])
next_index, revocations_received,
channel_reestablish, final_scriptpubkey);

/* We don't need this any more */
tal_free(final_scriptpubkey);

peer_billboard(true, "Negotiating closing fee between %"PRIu64
" and %"PRIu64" satoshi (ideal %"PRIu64")",
min_fee_to_accept, commitment_fee, offer[LOCAL]);
Expand Down Expand Up @@ -577,6 +602,11 @@ int main(int argc, char *argv[])
peer_billboard(true, "We agreed on a closing fee of %"PRIu64" satoshi",
offer[LOCAL]);

#if DEVELOPER
/* We don't listen for master commands, so always check memleak here */
closing_dev_memleak(ctx, scriptpubkey, funding_wscript);
#endif

/* We're done! */
/* Properly close the channel first. */
if (!socket_close(PEER_FD))
Expand Down
2 changes: 1 addition & 1 deletion lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -1455,7 +1455,7 @@ static void peer_memleak_req_next(struct command *cmd, struct channel *prev)
if (prev != NULL)
continue;

/* FIXME: handle closingd here */
/* Note: closingd does its own checking automatically */
if (streq(c->owner->name, "lightning_channeld")) {
subd_req(c, c->owner,
take(towire_channel_dev_memleak(NULL)),
Expand Down
11 changes: 11 additions & 0 deletions tests/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ def check_errors(request, err_count, msg):
if err_count:
raise ValueError("{} nodes had bad hsm requests".format(err_count))

for node in nf.nodes:
err_count += checkMemleak(node)
if err_count:
raise ValueError("{} nodes had memleak messages".format(err_count))

if not ok:
request.node.has_errors = True
raise Exception("At least one lightning exited with unexpected non-zero return code")
Expand Down Expand Up @@ -212,6 +217,12 @@ def checkBadHSMRequest(node):
return 0


def checkMemleak(node):
if node.daemon.is_in_log('MEMLEAK:'):
return 1
return 0


@pytest.fixture
def executor():
ex = futures.ThreadPoolExecutor(max_workers=20)
Expand Down

0 comments on commit 8fb1b60

Please sign in to comment.