From c956d9f5ebe21c1eea5c4c32545eb7260bc4fe55 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 15 Dec 2017 20:47:54 +1030 Subject: [PATCH] lightningd: tal memleak detection, dev-memleak command. This is a primitive mark-and-sweep-style garbage detector. The core is in common/ for later use by subdaemons, but for now it's just lightningd. We initialize it before most other allocations. We walk the tal tree to get all the pointers, then search the `ld` object for those pointers, recursing down. Some specific helpers are required for hashtables (which stash bits in the unused pointer bits, so won't be found). There's `notleak()` for annotating things that aren't leaks: things like globals and timers, and other semi-transients. Signed-off-by: Rusty Russell --- common/Makefile | 1 + common/memleak.c | 143 +++++++++++++++++++++++++++++ common/memleak.h | 45 +++++++++ lightningd/Makefile | 1 + lightningd/chaintopology.c | 21 +++++ lightningd/chaintopology.h | 3 + lightningd/htlc_end.c | 29 ++++++ lightningd/htlc_end.h | 8 ++ lightningd/lightningd.c | 20 ++-- lightningd/lightningd.h | 3 + lightningd/memdump.c | 61 ++++++++++++ lightningd/test/run-find_my_path.c | 6 ++ wallet/test/run-db.c | 4 + wallet/test/run-wallet.c | 4 + 14 files changed, 342 insertions(+), 7 deletions(-) create mode 100644 common/memleak.c create mode 100644 common/memleak.h diff --git a/common/Makefile b/common/Makefile index 9e059ea5d..49f6b2b7b 100644 --- a/common/Makefile +++ b/common/Makefile @@ -23,6 +23,7 @@ COMMON_SRC := \ common/json.c \ common/key_derive.c \ common/keyset.c \ + common/memleak.c \ common/msg_queue.c \ common/peer_failed.c \ common/permute_tx.c \ diff --git a/common/memleak.c b/common/memleak.c new file mode 100644 index 000000000..280be92cf --- /dev/null +++ b/common/memleak.c @@ -0,0 +1,143 @@ +#include +#include +#include +#include +#include + +#if DEVELOPER +static const void **notleaks; + +void *notleak_(const void *ptr) +{ + size_t nleaks; + + /* If we're not tracking, don't do anything. */ + if (!notleaks) + return cast_const(void *, ptr); + + /* FIXME: Doesn't work with reallocs, but tal_steal breaks lifetimes */ + nleaks = tal_count(notleaks); + tal_resize(¬leaks, nleaks+1); + notleaks[nleaks] = ptr; + return cast_const(void *, ptr); +} + +/* This only works if all objects have tal_len() */ +#ifndef CCAN_TAL_DEBUG +#error CCAN_TAL_DEBUG must be set +#endif + +static size_t hash_ptr(const void *elem, void *unused UNNEEDED) +{ + static struct siphash_seed seed; + return siphash24(&seed, &elem, sizeof(elem)); +} + +static bool pointer_referenced(struct htable *memtable, const void *p) +{ + return htable_del(memtable, hash_ptr(p, NULL), p); +} + +static void children_into_htable(const void *exclude, + struct htable *memtable, const tal_t *p) +{ + const tal_t *i; + + for (i = tal_first(p); i; i = tal_next(i)) { + if (p == exclude) + continue; + htable_add(memtable, hash_ptr(i, NULL), i); + children_into_htable(exclude, memtable, i); + } +} + +struct htable *memleak_enter_allocations(const tal_t *ctx, const void *exclude) +{ + struct htable *memtable = tal(ctx, struct htable); + htable_init(memtable, hash_ptr, NULL); + + /* First, add all pointers off NULL to table. */ + children_into_htable(exclude, memtable, NULL); + + tal_add_destructor(memtable, htable_clear); + return memtable; +} + +static void scan_for_pointers(struct htable *memtable, const tal_t *p) +{ + size_t i, n; + + /* Search for (aligned) pointers. */ + n = tal_len(p) / sizeof(void *); + for (i = 0; i < n; i++) { + void *ptr; + + memcpy(&ptr, (char *)p + i * sizeof(void *), sizeof(ptr)); + if (pointer_referenced(memtable, ptr)) + scan_for_pointers(memtable, ptr); + } +} + +void memleak_scan_region(struct htable *memtable, const void *ptr) +{ + pointer_referenced(memtable, ptr); + scan_for_pointers(memtable, ptr); +} + +void memleak_remove_referenced(struct htable *memtable, const void *root) +{ + /* Now delete the ones which are referenced. */ + memleak_scan_region(memtable, root); + memleak_scan_region(memtable, notleaks); + + /* Remove memtable itself */ + pointer_referenced(memtable, memtable); +} + +static void remove_with_children(struct htable *memtable, const tal_t *p) +{ + const tal_t *i; + + pointer_referenced(memtable, p); + for (i = tal_first(p); i; i = tal_next(i)) + remove_with_children(memtable, i); +} + +static bool ptr_match(const void *candidate, void *ptr) +{ + return candidate == ptr; +} + +const void *memleak_get(struct htable *memtable) +{ + struct htable_iter it; + const tal_t *i, *p; + + i = htable_first(memtable, &it); + if (!i) + return NULL; + + /* Delete from table (avoids parenting loops) */ + htable_delval(memtable, &it); + + /* Find ancestor, which is probably source of leak. */ + for (p = tal_parent(i); + htable_get(memtable, hash_ptr(p, NULL), ptr_match, p); + i = p, p = tal_parent(i)); + + /* Delete all children */ + remove_with_children(memtable, i); + + return i; +} + +void memleak_init(const tal_t *root) +{ + notleaks = tal_arr(NULL, const void *, 0); +} + +void memleak_cleanup(void) +{ + notleaks = tal_free(notleaks); +} +#endif /* DEVELOPER */ diff --git a/common/memleak.h b/common/memleak.h new file mode 100644 index 000000000..aa3a4ffbb --- /dev/null +++ b/common/memleak.h @@ -0,0 +1,45 @@ +#ifndef LIGHTNING_COMMON_MEMLEAK_H +#define LIGHTNING_COMMON_MEMLEAK_H +#include "config.h" +#include + +#if HAVE_TYPEOF +#define memleak_typeof(var) typeof(var) +#else +#define memleak_typeof(var) void * +#endif /* !HAVE_TYPEOF */ + +/* Mark a pointer as not being leaked. */ +#define notleak(p) ((memleak_typeof(p))notleak_(p)) + +#if DEVELOPER +void *notleak_(const void *ptr); + +struct htable; + +/* Initialize memleak detection, with this as the root */ +void memleak_init(const tal_t *root); + +/* Free memleak detection. */ +void memleak_cleanup(void); + +/* Allocate a htable with all the memory we've allocated. */ +struct htable *memleak_enter_allocations(const tal_t *ctx, const void *exclude); + +/* Remove any pointers to memory under root */ +void memleak_remove_referenced(struct htable *memtable, const void *root); + +/* Mark this pointer as being referenced, and search within for more. */ +void memleak_scan_region(struct htable *memtable, const void *p); + +/* Get (and remove) a leak from memtable, or NULL */ +const void *memleak_get(struct htable *memtable); + +#else /* ... !DEVELOPER */ +static inline void *notleak_(const void *ptr) +{ + return ptr; +} +#endif /* !DEVELOPER */ + +#endif /* LIGHTNING_COMMON_MEMLEAK_H */ diff --git a/lightningd/Makefile b/lightningd/Makefile index 1b1aed7b5..8c5fde517 100644 --- a/lightningd/Makefile +++ b/lightningd/Makefile @@ -28,6 +28,7 @@ LIGHTNINGD_COMMON_OBJS := \ common/io_debug.o \ common/key_derive.o \ common/json.o \ + common/memleak.o \ common/msg_queue.o \ common/permute_tx.o \ common/pseudorand.o \ diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index 87689f8c4..0b6befe71 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -675,6 +676,26 @@ static const struct json_command dev_setfees_command = { "Allows testing of feerate changes" }; AUTODATA(json_command, &dev_setfees_command); + +void chaintopology_mark_pointers_used(struct htable *memtable, + const struct chain_topology *topo) +{ + struct txwatch_hash_iter wit; + struct txwatch *w; + struct txowatch_hash_iter owit; + struct txowatch *ow; + + /* memleak can't see inside hash tables, so do them manually */ + for (w = txwatch_hash_first(&topo->txwatches, &wit); + w; + w = txwatch_hash_next(&topo->txwatches, &wit)) + memleak_scan_region(memtable, w); + + for (ow = txowatch_hash_first(&topo->txowatches, &owit); + ow; + ow = txowatch_hash_next(&topo->txowatches, &owit)) + memleak_scan_region(memtable, ow); +} #endif /* DEVELOPER */ /* On shutdown, peers get deleted last. That frees from our list, so diff --git a/lightningd/chaintopology.h b/lightningd/chaintopology.h index 7c47e2e54..09a4dbc3a 100644 --- a/lightningd/chaintopology.h +++ b/lightningd/chaintopology.h @@ -170,5 +170,8 @@ void notify_feerate_change(struct lightningd *ld); void json_dev_broadcast(struct command *cmd, struct chain_topology *topo, const char *buffer, const jsmntok_t *params); + +void chaintopology_mark_pointers_used(struct htable *memtable, + const struct chain_topology *topo); #endif #endif /* LIGHTNING_LIGHTNINGD_CHAINTOPOLOGY_H */ diff --git a/lightningd/htlc_end.c b/lightningd/htlc_end.c index e082a1d9a..7ce344eca 100644 --- a/lightningd/htlc_end.c +++ b/lightningd/htlc_end.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -166,3 +167,31 @@ struct htlc_out *new_htlc_out(const tal_t *ctx, return htlc_out_check(hout, "new_htlc_out"); } + +#if DEVELOPER +void htlc_inmap_mark_pointers_used(struct htable *memtable, + const struct htlc_in_map *map) +{ + struct htlc_in *hin; + struct htlc_in_map_iter it; + + /* memleak can't see inside hash tables, so do them manually */ + for (hin = htlc_in_map_first(map, &it); + hin; + hin = htlc_in_map_next(map, &it)) + memleak_scan_region(memtable, hin); +} + +void htlc_outmap_mark_pointers_used(struct htable *memtable, + const struct htlc_out_map *map) +{ + struct htlc_out *hout; + struct htlc_out_map_iter it; + + /* memleak can't see inside hash tables, so do them manually */ + for (hout = htlc_out_map_first(map, &it); + hout; + hout = htlc_out_map_next(map, &it)) + memleak_scan_region(memtable, hout); +} +#endif /* DEVELOPER */ diff --git a/lightningd/htlc_end.h b/lightningd/htlc_end.h index c7f0cfcdf..176673788 100644 --- a/lightningd/htlc_end.h +++ b/lightningd/htlc_end.h @@ -143,4 +143,12 @@ void connect_htlc_out(struct htlc_out_map *map, struct htlc_out *hout); struct htlc_out *htlc_out_check(const struct htlc_out *hout, const char *abortstr); struct htlc_in *htlc_in_check(const struct htlc_in *hin, const char *abortstr); + +#if DEVELOPER +struct htable; +void htlc_inmap_mark_pointers_used(struct htable *memtable, + const struct htlc_in_map *map); +void htlc_outmap_mark_pointers_used(struct htable *memtable, + const struct htlc_out_map *map); +#endif /* DEVELOPER */ #endif /* LIGHTNING_LIGHTNINGD_HTLC_END_H */ diff --git a/lightningd/lightningd.c b/lightningd/lightningd.c index 09bc5c63a..d4c40672c 100644 --- a/lightningd/lightningd.c +++ b/lightningd/lightningd.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -65,6 +66,14 @@ static struct lightningd *new_lightningd(const tal_t *ctx, { struct lightningd *ld = tal(ctx, struct lightningd); +#if DEVELOPER + ld->dev_debug_subdaemon = NULL; + ld->dev_disconnect_fd = -1; + ld->dev_hsm_seed = NULL; + ld->dev_subdaemon_fail = false; + memleak_init(ld); +#endif + list_head_init(&ld->peers); htlc_in_map_init(&ld->htlcs_in); htlc_out_map_init(&ld->htlcs_out); @@ -82,13 +91,6 @@ static struct lightningd *new_lightningd(const tal_t *ctx, /* FIXME: Move into invoice daemon. */ ld->invoices = invoices_init(ld); -#if DEVELOPER - ld->dev_debug_subdaemon = NULL; - ld->dev_disconnect_fd = -1; - ld->dev_hsm_seed = NULL; - ld->dev_subdaemon_fail = false; -#endif - return ld; } @@ -349,5 +351,9 @@ int main(int argc, char *argv[]) tal_free(ld); opt_free_table(); tal_free(log_book); + +#if DEVELOPER + memleak_cleanup(); +#endif return 0; } diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h index fd161f7c4..597cac4c4 100644 --- a/lightningd/lightningd.h +++ b/lightningd/lightningd.h @@ -147,6 +147,9 @@ struct lightningd { /* If we have --dev-fail-on-subdaemon-fail */ bool dev_subdaemon_fail; + + /* Things we've marked as not leaking. */ + const void **notleaks; #endif /* DEVELOPER */ }; diff --git a/lightningd/memdump.c b/lightningd/memdump.c index 22f9bbdcf..c8e2cca5b 100644 --- a/lightningd/memdump.c +++ b/lightningd/memdump.c @@ -1,6 +1,10 @@ /* Only possible if we're in developer mode. */ #ifdef DEVELOPER +#include +#include #include +#include +#include #include static void json_add_ptr(struct json_result *response, const char *name, @@ -60,4 +64,61 @@ static const struct json_command dev_memdump_command = { "Debugging tool for memory leaks" }; AUTODATA(json_command, &dev_memdump_command); + +static void scan_mem(struct command *cmd, + struct json_result *response, + struct lightningd *ld) +{ + struct htable *memtable; + const tal_t *i; + + /* Enter everything, except this cmd. */ + memtable = memleak_enter_allocations(cmd, cmd); + + /* First delete known false positives. */ + chaintopology_mark_pointers_used(memtable, ld->topology); + htlc_inmap_mark_pointers_used(memtable, &ld->htlcs_in); + htlc_outmap_mark_pointers_used(memtable, &ld->htlcs_out); + + /* Now delete ld and those which it has pointers to. */ + memleak_remove_referenced(memtable, ld); + + json_array_start(response, "leaks"); + while ((i = memleak_get(memtable)) != NULL) { + const tal_t *p; + + json_object_start(response, NULL); + json_add_ptr(response, "value", i); + if (tal_name(i)) + json_add_string(response, "label", tal_name(i)); + + json_array_start(response, "parents"); + for (p = tal_parent(i); p; p = tal_parent(p)) { + json_add_string(response, NULL, tal_name(p)); + p = tal_parent(p); + } + json_array_end(response); + json_object_end(response); + } + json_array_end(response); +} + +static void json_memleak(struct command *cmd, + const char *buffer UNNEEDED, + const jsmntok_t *params UNNEEDED) +{ + struct json_result *response = new_json_result(cmd); + + scan_mem(cmd, response, cmd->ld); + + command_success(cmd, response); +} + +static const struct json_command dev_memleak_command = { + "dev-memleak", + json_memleak, + "Dump the memory objects unreferenced", + "Debugging tool for memory leaks" +}; +AUTODATA(json_command, &dev_memleak_command); #endif /* DEVELOPER */ diff --git a/lightningd/test/run-find_my_path.c b/lightningd/test/run-find_my_path.c index d76d90987..3645585f0 100644 --- a/lightningd/test/run-find_my_path.c +++ b/lightningd/test/run-find_my_path.c @@ -44,6 +44,12 @@ struct invoices *invoices_init(const tal_t *ctx UNNEEDED) void log_(struct log *log UNNEEDED, enum log_level level UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "log_ called!\n"); abort(); } +/* Generated stub for memleak_cleanup */ +void memleak_cleanup(void) +{ fprintf(stderr, "memleak_cleanup called!\n"); abort(); } +/* Generated stub for memleak_init */ +void memleak_init(const tal_t *root UNNEEDED) +{ fprintf(stderr, "memleak_init called!\n"); abort(); } /* Generated stub for new_log */ struct log *new_log(const tal_t *ctx UNNEEDED, struct log_book *record UNNEEDED, const char *fmt UNNEEDED, ...) { fprintf(stderr, "new_log called!\n"); abort(); } diff --git a/wallet/test/run-db.c b/wallet/test/run-db.c index 0c6de001e..a21b78303 100644 --- a/wallet/test/run-db.c +++ b/wallet/test/run-db.c @@ -7,10 +7,14 @@ static void db_fatal(const char *fmt, ...); #include "test_utils.h" +#include #include #include /* AUTOGENERATED MOCKS START */ +/* Generated stub for memleak_scan_region */ +void memleak_scan_region(struct htable *memtable UNNEEDED, const void *p UNNEEDED) +{ fprintf(stderr, "memleak_scan_region called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ static char *db_err; diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 7d6b15eea..48aac9029 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -10,11 +10,15 @@ static void wallet_fatal(const char *fmt, ...); #include #include +#include #include #include #include /* AUTOGENERATED MOCKS START */ +/* Generated stub for memleak_scan_region */ +void memleak_scan_region(struct htable *memtable UNNEEDED, const void *p UNNEEDED) +{ fprintf(stderr, "memleak_scan_region called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ static char *wallet_err;