diff --git a/lightningd/onchain_control.c b/lightningd/onchain_control.c index 13759b4cb..4f70a1577 100644 --- a/lightningd/onchain_control.c +++ b/lightningd/onchain_control.c @@ -327,6 +327,8 @@ static unsigned int onchain_msg(struct subd *sd, const u8 *msg, const int *fds U case WIRE_ONCHAIN_DEPTH: case WIRE_ONCHAIN_HTLC: case WIRE_ONCHAIN_KNOWN_PREIMAGE: + case WIRE_ONCHAIN_DEV_MEMLEAK: + case WIRE_ONCHAIN_DEV_MEMLEAK_REPLY: break; } diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 7939f002b..0de890f6c 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -1399,23 +1399,41 @@ AUTODATA(json_command, &dev_forget_channel_command); /* Mutual recursion */ static void peer_memleak_req_next(struct command *cmd, struct channel *prev); +static void peer_memleak_req_done(struct subd *subd, bool found_leak, + struct command *cmd) +{ + struct channel *c = subd->channel; + + if (found_leak) + peer_memleak_done(cmd, subd); + else + peer_memleak_req_next(cmd, c); +} + static void channeld_memleak_req_done(struct subd *channeld, const u8 *msg, const int *fds UNUSED, struct command *cmd) { - struct channel *c = channeld->channel; bool found_leak; if (!fromwire_channel_dev_memleak_reply(msg, &found_leak)) { command_fail(cmd, LIGHTNINGD, "Bad channel_dev_memleak"); return; } + peer_memleak_req_done(channeld, found_leak, cmd); +} + +static void onchaind_memleak_req_done(struct subd *onchaind, + const u8 *msg, const int *fds UNUSED, + struct command *cmd) +{ + bool found_leak; - if (found_leak) { - peer_memleak_done(cmd, channeld); + if (!fromwire_onchain_dev_memleak_reply(msg, &found_leak)) { + command_fail(cmd, LIGHTNINGD, "Bad onchain_dev_memleak"); return; } - peer_memleak_req_next(cmd, c); + peer_memleak_req_done(onchaind, found_leak, cmd); } static void peer_memleak_req_next(struct command *cmd, struct channel *prev) @@ -1437,7 +1455,6 @@ static void peer_memleak_req_next(struct command *cmd, struct channel *prev) if (prev != NULL) continue; - /* FIXME: handle onchaind here */ /* FIXME: handle closingd here */ if (streq(c->owner->name, "lightning_channeld")) { subd_req(c, c->owner, @@ -1445,6 +1462,12 @@ static void peer_memleak_req_next(struct command *cmd, struct channel *prev) -1, 0, channeld_memleak_req_done, cmd); return; } + if (streq(c->owner->name, "lightning_onchaind")) { + subd_req(c, c->owner, + take(towire_onchain_dev_memleak(NULL)), + -1, 0, onchaind_memleak_req_done, cmd); + return; + } } } peer_memleak_done(cmd, NULL); diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index c758fde1a..9c8535f6a 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -85,6 +85,9 @@ bool fromwire_hsm_sign_commitment_tx_reply(const void *p UNNEEDED, secp256k1_ecd /* Generated stub for fromwire_hsm_sign_invoice_reply */ bool fromwire_hsm_sign_invoice_reply(const void *p UNNEEDED, secp256k1_ecdsa_recoverable_signature *sig UNNEEDED) { fprintf(stderr, "fromwire_hsm_sign_invoice_reply called!\n"); abort(); } +/* Generated stub for fromwire_onchain_dev_memleak_reply */ +bool fromwire_onchain_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED) +{ fprintf(stderr, "fromwire_onchain_dev_memleak_reply called!\n"); abort(); } /* Generated stub for get_block_height */ u32 get_block_height(const struct chain_topology *topo UNNEEDED) { fprintf(stderr, "get_block_height called!\n"); abort(); } @@ -386,6 +389,9 @@ u8 *towire_hsm_sign_commitment_tx(const tal_t *ctx UNNEEDED, const struct pubkey /* Generated stub for towire_hsm_sign_invoice */ u8 *towire_hsm_sign_invoice(const tal_t *ctx UNNEEDED, const u8 *u5bytes UNNEEDED, const u8 *hrp UNNEEDED) { fprintf(stderr, "towire_hsm_sign_invoice called!\n"); abort(); } +/* Generated stub for towire_onchain_dev_memleak */ +u8 *towire_onchain_dev_memleak(const tal_t *ctx UNNEEDED) +{ fprintf(stderr, "towire_onchain_dev_memleak called!\n"); abort(); } /* Generated stub for txfilter_add_scriptpubkey */ void txfilter_add_scriptpubkey(struct txfilter *filter UNNEEDED, const u8 *script TAKES UNNEEDED) { fprintf(stderr, "txfilter_add_scriptpubkey called!\n"); abort(); } diff --git a/onchaind/onchain_wire.csv b/onchaind/onchain_wire.csv index f844eaea5..ac97561cd 100644 --- a/onchaind/onchain_wire.csv +++ b/onchaind/onchain_wire.csv @@ -90,3 +90,9 @@ onchain_add_utxo,,prev_out_index,u32 onchain_add_utxo,,per_commit_point,struct pubkey onchain_add_utxo,,value,u64 onchain_add_utxo,,blockheight,u32 + +# master -> onchaind: do you have a memleak? +onchain_dev_memleak,5033 + +onchain_dev_memleak_reply,5133 +onchain_dev_memleak_reply,,leak,bool diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c index c5065bc97..820256c5c 100644 --- a/onchaind/onchaind.c +++ b/onchaind/onchaind.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -91,8 +92,8 @@ struct tracked_output { u64 satoshi; enum output_type output_type; - /* If it is an HTLC, these are non-NULL */ - const struct htlc_stub *htlc; + /* If it is an HTLC, this is set, wscript is non-NULL. */ + struct htlc_stub htlc; const u8 *wscript; /* If it's an HTLC off our unilateral, this is their sig for htlc_tx */ @@ -397,8 +398,9 @@ static struct tracked_output * out->output_type = output_type; out->proposal = NULL; out->resolved = NULL; - out->htlc = htlc; - out->wscript = wscript; + if (htlc) + out->htlc = *htlc; + out->wscript = tal_steal(out, wscript); out->remote_htlc_sig = remote_htlc_sig; *tal_arr_expand(outs) = out; @@ -811,14 +813,14 @@ static void handle_htlc_onchain_fulfill(struct tracked_output *out, sha256(&sha, &preimage, sizeof(preimage)); ripemd160(&ripemd, &sha, sizeof(sha)); - if (!ripemd160_eq(&ripemd, &out->htlc->ripemd)) + if (!ripemd160_eq(&ripemd, &out->htlc.ripemd)) status_failed(STATUS_FAIL_INTERNAL_ERROR, "%s/%s spent with bad preimage %s (ripemd not %s)", tx_type_name(out->tx_type), output_type_name(out->output_type), type_to_string(tmpctx, struct preimage, &preimage), type_to_string(tmpctx, struct ripemd160, - &out->htlc->ripemd)); + &out->htlc.ripemd)); /* Tell master we found a preimage. */ status_trace("%s/%s gave us preimage %s", @@ -1035,7 +1037,7 @@ static void update_resolution_depth(struct tracked_output *out, u32 depth) tx_type_name(out->tx_type), output_type_name(out->output_type), depth); - msg = towire_onchain_htlc_timeout(out, out->htlc); + msg = towire_onchain_htlc_timeout(out, &out->htlc); wire_sync_write(REQ_FD, take(msg)); } out->resolved->depth = depth; @@ -1123,7 +1125,7 @@ static void handle_preimage(struct tracked_output **outs, if (outs[i]->output_type != THEIR_HTLC) continue; - if (!ripemd160_eq(&outs[i]->htlc->ripemd, &ripemd)) + if (!ripemd160_eq(&outs[i]->htlc.ripemd, &ripemd)) continue; /* Too late? */ @@ -1194,6 +1196,55 @@ static void handle_preimage(struct tracked_output **outs, } } +#if DEVELOPER +static void memleak_remove_globals(struct htable *memtable, const tal_t *topctx) +{ + /* memleak_scan_region is overkill if these are simple pointers to + * objects which don't contain pointers, but it works. */ + if (keyset) + memleak_scan_region(memtable, keyset, sizeof(*keyset)); + + if (remote_per_commitment_point) + memleak_scan_region(memtable, remote_per_commitment_point, + sizeof(*remote_per_commitment_point)); + + if (remote_per_commitment_secret) + memleak_scan_region(memtable, remote_per_commitment_secret, + sizeof(*remote_per_commitment_secret)); + + /* top-level context args */ + memleak_scan_region(memtable, topctx, 0); + + memleak_scan_region(memtable, missing_htlc_msgs, + tal_bytelen(missing_htlc_msgs)); +} + +static bool handle_dev_memleak(struct tracked_output **outs, const u8 *msg) +{ + struct htable *memtable; + bool found_leak; + + if (!fromwire_onchain_dev_memleak(msg)) + return false; + + memtable = memleak_enter_allocations(tmpctx, msg, msg); + /* Top-level context is parent of outs */ + memleak_remove_globals(memtable, tal_parent(outs)); + memleak_remove_referenced(memtable, outs); + + found_leak = dump_memleak(memtable); + wire_sync_write(REQ_FD, + take(towire_onchain_dev_memleak_reply(NULL, + found_leak))); + return true; +} +#else +static bool handle_dev_memleak(struct tracked_output **outs, const u8 *msg) +{ + return false; +} +#endif /* !DEVELOPER */ + /* BOLT #5: * * A node: @@ -1224,7 +1275,7 @@ static void wait_for_resolved(struct tracked_output **outs) output_spent(&outs, tx, input_num, tx_blockheight); else if (fromwire_onchain_known_preimage(msg, &preimage)) handle_preimage(outs, &preimage); - else + else if (!handle_dev_memleak(outs, msg)) master_badmsg(-1, msg); billboard_update(outs); @@ -1715,8 +1766,8 @@ static void handle_our_unilateral(const struct bitcoin_tx *tx, which_htlc = resolve_their_htlc(out, matches, htlcs, htlc_scripts); } - out->htlc = &htlcs[which_htlc]; - out->wscript = htlc_scripts[which_htlc]; + out->htlc = htlcs[which_htlc]; + out->wscript = tal_steal(out, htlc_scripts[which_htlc]); /* Each of these consumes one HTLC signature */ remote_htlc_sigs++; @@ -2217,8 +2268,8 @@ static void handle_their_unilateral(const struct bitcoin_tx *tx, which_htlc = resolve_their_htlc(out, matches, htlcs, htlc_scripts); } - out->htlc = &htlcs[which_htlc]; - out->wscript = htlc_scripts[which_htlc]; + out->htlc = htlcs[which_htlc]; + out->wscript = tal_steal(out, htlc_scripts[which_htlc]); htlc_scripts[which_htlc] = NULL; } @@ -2342,7 +2393,7 @@ int main(int argc, char *argv[]) missing_htlc_msgs = tal_arr(ctx, u8 *, 0); msg = wire_sync_read(tmpctx, REQ_FD); - if (!fromwire_onchain_init(ctx, msg, + if (!fromwire_onchain_init(tmpctx, msg, &shachain, &funding_amount_satoshi, &old_remote_per_commit_point, @@ -2370,11 +2421,13 @@ int main(int argc, char *argv[]) } bitcoin_txid(tx, &txid); + /* We need to keep tx around, but there's only one: not really a leak */ + tal_steal(ctx, notleak(tx)); /* FIXME: Filter as we go, don't load them all into mem! */ - htlcs = tal_arr(ctx, struct htlc_stub, num_htlcs); - tell_if_missing = tal_arr(ctx, bool, num_htlcs); - tell_immediately = tal_arr(ctx, bool, num_htlcs); + htlcs = tal_arr(tmpctx, struct htlc_stub, num_htlcs); + tell_if_missing = tal_arr(htlcs, bool, num_htlcs); + tell_immediately = tal_arr(htlcs, bool, num_htlcs); if (!htlcs || !tell_if_missing || !tell_immediately) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Can't allocate %"PRIu64" htlcs", num_htlcs); diff --git a/onchaind/test/run-grind_feerate.c b/onchaind/test/run-grind_feerate.c index 988d9801e..68e7163ab 100644 --- a/onchaind/test/run-grind_feerate.c +++ b/onchaind/test/run-grind_feerate.c @@ -33,6 +33,9 @@ bool fromwire_hsm_sign_tx_reply(const void *p UNNEEDED, secp256k1_ecdsa_signatur /* Generated stub for fromwire_onchain_depth */ bool fromwire_onchain_depth(const void *p UNNEEDED, struct bitcoin_txid *txid UNNEEDED, u32 *depth UNNEEDED) { fprintf(stderr, "fromwire_onchain_depth called!\n"); abort(); } +/* Generated stub for fromwire_onchain_dev_memleak */ +bool fromwire_onchain_dev_memleak(const void *p UNNEEDED) +{ fprintf(stderr, "fromwire_onchain_dev_memleak called!\n"); abort(); } /* Generated stub for fromwire_onchain_htlc */ bool fromwire_onchain_htlc(const void *p UNNEEDED, struct htlc_stub *htlc UNNEEDED, bool *tell_if_missing UNNEEDED, bool *tell_immediately UNNEEDED) { fprintf(stderr, "fromwire_onchain_htlc called!\n"); abort(); } @@ -129,6 +132,9 @@ u8 *towire_onchain_all_irrevocably_resolved(const tal_t *ctx UNNEEDED) /* Generated stub for towire_onchain_broadcast_tx */ u8 *towire_onchain_broadcast_tx(const tal_t *ctx UNNEEDED, const struct bitcoin_tx *tx UNNEEDED) { fprintf(stderr, "towire_onchain_broadcast_tx called!\n"); abort(); } +/* Generated stub for towire_onchain_dev_memleak_reply */ +u8 *towire_onchain_dev_memleak_reply(const tal_t *ctx UNNEEDED, bool leak UNNEEDED) +{ fprintf(stderr, "towire_onchain_dev_memleak_reply called!\n"); abort(); } /* Generated stub for towire_onchain_extracted_preimage */ u8 *towire_onchain_extracted_preimage(const tal_t *ctx UNNEEDED, const struct preimage *preimage UNNEEDED) { fprintf(stderr, "towire_onchain_extracted_preimage called!\n"); abort(); } @@ -152,6 +158,27 @@ bool wire_sync_write(int fd UNNEEDED, const void *msg TAKES UNNEEDED) { fprintf(stderr, "wire_sync_write called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ +#if DEVELOPER +/* Generated stub for dump_memleak */ +bool dump_memleak(struct htable *memtable UNNEEDED) +{ fprintf(stderr, "dump_memleak called!\n"); abort(); } +/* Generated stub for memleak_enter_allocations */ +struct htable *memleak_enter_allocations(const tal_t *ctx UNNEEDED, + const void *exclude1 UNNEEDED, + const void *exclude2 UNNEEDED) +{ fprintf(stderr, "memleak_enter_allocations called!\n"); abort(); } +/* Generated stub for memleak_remove_referenced */ +void memleak_remove_referenced(struct htable *memtable UNNEEDED, const void *root UNNEEDED) +{ fprintf(stderr, "memleak_remove_referenced called!\n"); abort(); } +/* Generated stub for memleak_scan_region */ +void memleak_scan_region(struct htable *memtable UNNEEDED, + const void *p UNNEEDED, size_t bytelen UNNEEDED) +{ fprintf(stderr, "memleak_scan_region called!\n"); abort(); } +/* Generated stub for notleak_ */ +void *notleak_(const void *ptr UNNEEDED, bool plus_children UNNEEDED) +{ fprintf(stderr, "notleak_ called!\n"); abort(); } +#endif + int main(int argc, char *argv[]) { setup_locale(); diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index fab5a1204..ff01dc51c 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -94,6 +94,9 @@ bool fromwire_gossip_get_channel_peer_reply(const tal_t *ctx UNNEEDED, const voi /* Generated stub for fromwire_hsm_sign_commitment_tx_reply */ bool fromwire_hsm_sign_commitment_tx_reply(const void *p UNNEEDED, secp256k1_ecdsa_signature *sig UNNEEDED) { fprintf(stderr, "fromwire_hsm_sign_commitment_tx_reply called!\n"); abort(); } +/* Generated stub for fromwire_onchain_dev_memleak_reply */ +bool fromwire_onchain_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED) +{ fprintf(stderr, "fromwire_onchain_dev_memleak_reply called!\n"); abort(); } /* Generated stub for get_block_height */ u32 get_block_height(const struct chain_topology *topo UNNEEDED) { fprintf(stderr, "get_block_height called!\n"); abort(); } @@ -451,6 +454,9 @@ u8 *towire_gossip_get_channel_peer(const tal_t *ctx UNNEEDED, const struct short /* Generated stub for towire_hsm_sign_commitment_tx */ u8 *towire_hsm_sign_commitment_tx(const tal_t *ctx UNNEEDED, const struct pubkey *peer_id UNNEEDED, u64 channel_dbid UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, const struct pubkey *remote_funding_key UNNEEDED, u64 funding_amount UNNEEDED) { fprintf(stderr, "towire_hsm_sign_commitment_tx called!\n"); abort(); } +/* Generated stub for towire_onchain_dev_memleak */ +u8 *towire_onchain_dev_memleak(const tal_t *ctx UNNEEDED) +{ fprintf(stderr, "towire_onchain_dev_memleak called!\n"); abort(); } /* Generated stub for towire_onchain_known_preimage */ u8 *towire_onchain_known_preimage(const tal_t *ctx UNNEEDED, const struct preimage *preimage UNNEEDED) { fprintf(stderr, "towire_onchain_known_preimage called!\n"); abort(); }