memleak: handle libwally allocations better.

Things allocated by libwally all get the tal_name "wally_tal",
which cost me a few hours trying to find a leak.

In the case where we're making one of the allocations the parent
of the others (e.g. a wally_psbt), we can do better: supply a name
for the tal_wally_end().

So I add a new tal_wally_end_onto() which does the standard
tal_steal() trick, and also changes the (typechecked!) name.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell
2022-03-20 12:59:27 +10:30
parent 8994b8dade
commit 65be18d355
9 changed files with 55 additions and 33 deletions

View File

@@ -26,7 +26,7 @@ static char *to_base58(const tal_t *ctx, u8 version,
total_length, BASE58_FLAG_CHECKSUM, &out)
!= WALLY_OK)
out = NULL;
tal_wally_end(tal_steal(ctx, out));
tal_wally_end_onto(ctx, out, char);
return out;
}

View File

@@ -29,7 +29,7 @@ static struct wally_psbt *init_psbt(const tal_t *ctx, size_t num_inputs, size_t
wally_err = wally_psbt_init_alloc(0, num_inputs, num_outputs, 0, &psbt);
assert(wally_err == WALLY_OK);
tal_add_destructor(psbt, psbt_destroy);
tal_wally_end(tal_steal(ctx, psbt));
tal_wally_end_onto(ctx, psbt, struct wally_psbt);
return psbt;
}
@@ -63,7 +63,7 @@ struct wally_psbt *clone_psbt(const tal_t *ctx, struct wally_psbt *psbt)
tal_wally_start();
if (wally_psbt_clone_alloc(psbt, 0, &clone) != WALLY_OK)
abort();
tal_wally_end(tal_steal(ctx, clone));
tal_wally_end_onto(ctx, clone, struct wally_psbt);
return clone;
}
@@ -634,7 +634,7 @@ bool psbt_finalize(struct wally_psbt *psbt)
wally_tx_witness_stack_add(stack,
input->witness_script,
input->witness_script_len);
input->final_witness = stack;
wally_psbt_input_set_final_witness(input, stack);
}
ok = (wally_psbt_finalize(psbt) == WALLY_OK);
@@ -656,7 +656,7 @@ struct wally_tx *psbt_final_tx(const tal_t *ctx, const struct wally_psbt *psbt)
else
wtx = NULL;
tal_wally_end(tal_steal(ctx, wtx));
tal_wally_end_onto(ctx, wtx, struct wally_tx);
return wtx;
}
@@ -672,7 +672,7 @@ struct wally_psbt *psbt_from_b64(const tal_t *ctx,
tal_add_destructor(psbt, psbt_destroy);
else
psbt = NULL;
tal_wally_end(tal_steal(ctx, psbt));
tal_wally_end_onto(ctx, psbt, struct wally_psbt);
return psbt;
}
@@ -685,7 +685,7 @@ char *psbt_to_b64(const tal_t *ctx, const struct wally_psbt *psbt)
tal_wally_start();
ret = wally_psbt_to_base64(psbt, 0, &serialized_psbt);
assert(ret == WALLY_OK);
tal_wally_end(tal_steal(ctx, serialized_psbt));
tal_wally_end_onto(ctx, serialized_psbt, char);
return serialized_psbt;
}
@@ -723,7 +723,7 @@ struct wally_psbt *psbt_from_bytes(const tal_t *ctx, const u8 *bytes,
tal_add_destructor(psbt, psbt_destroy);
else
psbt = NULL;
tal_wally_end(tal_steal(ctx, psbt));
tal_wally_end_onto(ctx, psbt, struct wally_psbt);
return psbt;
}
@@ -798,7 +798,7 @@ void psbt_txid(const tal_t *ctx,
wally_tx_set_input_script(tx, i, script, tal_bytelen(script));
}
}
tal_wally_end(tal_steal(ctx, tx));
tal_wally_end_onto(ctx, tx, struct wally_tx);
wally_txid(tx, txid);
if (wtx)

View File

@@ -54,7 +54,7 @@ struct wally_tx_output *wally_tx_output(const tal_t *ctx,
}
done:
tal_wally_end(tal_steal(ctx, output));
tal_wally_end_onto(ctx, output, struct wally_tx_output);
return output;
}
@@ -520,7 +520,7 @@ struct bitcoin_tx *bitcoin_tx(const tal_t *ctx,
wally_tx_init_alloc(WALLY_TX_VERSION_2, nlocktime, input_count, output_count,
&tx->wtx);
tal_add_destructor(tx, bitcoin_tx_destroy);
tal_wally_end(tal_steal(tx, tx->wtx));
tal_wally_end_onto(tx, tx->wtx, struct wally_tx);
tx->chainparams = chainparams;
tx->psbt = new_psbt(tx, tx->wtx);
@@ -548,7 +548,7 @@ struct bitcoin_tx *bitcoin_tx_with_psbt(const tal_t *ctx, struct wally_psbt *psb
tal_wally_start();
if (wally_tx_clone_alloc(psbt->tx, 0, &tx->wtx) != WALLY_OK)
tx->wtx = NULL;
tal_wally_end(tal_steal(tx, tx->wtx));
tal_wally_end_onto(tx, tx->wtx, struct wally_tx);
if (!tx->wtx)
return tal_free(tx);
}
@@ -574,7 +574,7 @@ static struct wally_tx *pull_wtx(const tal_t *ctx,
fromwire_fail(cursor, max);
wtx = tal_free(wtx);
}
tal_wally_end(tal_steal(ctx, wtx));
tal_wally_end_onto(ctx, wtx, struct wally_tx);
if (wtx) {
size_t wsize;

View File

@@ -154,7 +154,7 @@ fromwire_wally_tx_witness_stack(const tal_t *ctx,
tal_add_destructor(ws, destroy_wally_tx_witness_stack);
out:
tal_wally_end(tal_steal(ctx, ws));
tal_wally_end_onto(ctx, ws, struct wally_tx_witness_stack);
return ws;
}
@@ -252,7 +252,7 @@ static struct wally_tx_input *fromwire_wally_tx_input(const tal_t *ctx,
tal_add_destructor(in, destroy_wally_tx_input);
}
tal_wally_end(tal_steal(ctx, in));
tal_wally_end_onto(ctx, in, struct wally_tx_input);
return in;
}
@@ -310,7 +310,7 @@ static struct wally_tx_output *fromwire_wally_tx_output(const tal_t *ctx,
} else {
tal_add_destructor(out, destroy_wally_tx_output);
}
tal_wally_end(tal_steal(ctx, out));
tal_wally_end_onto(ctx, out, struct wally_tx_output);
return out;
}

View File

@@ -34,20 +34,29 @@ void tal_wally_end(const tal_t *parent)
{
tal_t *p;
while ((p = tal_first(wally_tal_ctx)) != NULL) {
if (p != parent) {
/* Refuse to make a loop! */
assert(p != parent);
#if DEVELOPER
/* Don't steal backtrace from wally_tal_ctx! */
if (tal_name(p) && streq(tal_name(p), "backtrace")) {
tal_free(p);
continue;
}
#endif /* DEVELOPER */
tal_steal(parent, p);
/* Don't steal backtrace from wally_tal_ctx! */
if (tal_name(p) && streq(tal_name(p), "backtrace")) {
tal_free(p);
continue;
}
#endif /* DEVELOPER */
tal_steal(parent, p);
}
wally_tal_ctx = tal_free(wally_tal_ctx);
}
void tal_wally_end_onto_(const tal_t *parent,
tal_t *from_wally,
const char *from_wally_name)
{
if (from_wally)
tal_set_name_(from_wally, from_wally_name, 1);
tal_wally_end(tal_steal(parent, from_wally));
}
#if DEVELOPER
/* If you've got a softref, we assume no reallocs. */
static void dont_move_softref(tal_t *ctx, enum tal_notify_type ntype, void *info)

View File

@@ -105,10 +105,21 @@ void clean_tmpctx(void);
/* Call this before any libwally function which allocates. */
void tal_wally_start(void);
/* Then call this to reparent everything onto this parent (which must
* have been tal_steal() if it was allocated by libwally here) */
/* Then call this to reparent everything onto this parent */
void tal_wally_end(const tal_t *parent);
/* ... or this if you want to reparent onto something which is
* allocated by libwally here. Fixes up this from_wally obj to have a
* proper tal_name, too! */
#define tal_wally_end_onto(parent, from_wally, type) \
tal_wally_end_onto_((parent), \
(from_wally) + 0*sizeof((from_wally) == (type *)0), \
stringify(type))
void tal_wally_end_onto_(const tal_t *parent,
tal_t *from_wally,
const char *from_wally_name);
/* Define sha256_eq. */
STRUCTEQ_DEF(sha256, 0, u);

View File

@@ -1470,7 +1470,9 @@ static void send_funding_tx(struct channel *channel,
wally_tx_clone_alloc(wtx, 0,
cast_const2(struct wally_tx **,
&cs->wtx));
tal_wally_end(tal_steal(cs, cs->wtx));
tal_wally_end_onto(cs,
cast_const(struct wally_tx *,
cs->wtx), struct wally_tx);
}
wally_txid(wtx, &txid);
@@ -2405,7 +2407,7 @@ json_openchannel_signed(struct command *cmd,
/* Make memleak happy, (otherwise cleaned up with `cmd`) */
tal_free(psbt);
tal_wally_end(tal_steal(inflight, inflight->funding_psbt));
tal_wally_end_onto(inflight, inflight->funding_psbt, struct wally_psbt);
/* Update the PSBT on disk */
wallet_inflight_save(cmd->ld->wallet, inflight);

View File

@@ -179,7 +179,7 @@ mfc_cleanup_psbt(struct command *cmd,
tal_wally_start();
if (wally_psbt_clone_alloc(psbt, 0, &pruned_psbt) != WALLY_OK)
abort();
tal_wally_end(tal_steal(NULL, pruned_psbt));
tal_wally_end_onto(NULL, pruned_psbt, struct wally_psbt);
for (size_t i = pruned_psbt->num_inputs - 1;
i < pruned_psbt->num_inputs;

View File

@@ -76,7 +76,7 @@ static bool update_parent_psbt(const tal_t *ctx,
tal_wally_start();
if (wally_psbt_clone_alloc(*parent_psbt, 0, &clone) != WALLY_OK)
abort();
tal_wally_end(tal_steal(ctx, clone));
tal_wally_end_onto(ctx, clone, struct wally_psbt);
/* This makes it such that we can reparent/steal added
* inputs/outputs without impacting the 'original'. We
@@ -261,7 +261,7 @@ static bool update_node_psbt(const tal_t *ctx,
/* Only failure is alloc */
if (wally_psbt_clone_alloc(parent_psbt, 0, &clone) != WALLY_OK)
abort();
tal_wally_end(tal_steal(ctx, clone));
tal_wally_end_onto(ctx, clone, struct wally_psbt);
/* For every peer's input/output, flip the serial id
* on the clone. They should all be present. */
@@ -958,7 +958,7 @@ openchannel_init_ok(struct command *cmd,
* logic in `perform_openchannel_update` the same. */
tal_wally_start();
wally_psbt_clone_alloc(dest->updated_psbt, 0, &dest->psbt);
tal_wally_end(tal_steal(mfc, dest->updated_psbt));
tal_wally_end_onto(mfc, dest->updated_psbt, struct wally_psbt);
return openchannel_init_done(dest);
}
@@ -998,7 +998,7 @@ openchannel_init_dest(struct multifundchannel_destination *dest)
/* Copy the original parent down */
tal_wally_start();
wally_psbt_clone_alloc(mfc->psbt, 0, &dest->psbt);
tal_wally_end(tal_steal(mfc, dest->psbt));
tal_wally_end_onto(mfc, dest->psbt, struct wally_psbt);
json_add_psbt(req->js, "initialpsbt", dest->psbt);
if (mfc->cmtmt_feerate_str)