common/memleak: add dynamic hooks for assisting memleak.

Rather than reaching into data structures, let them register their own
callbacks.  This avoids us having to expose "memleak_remove_xxx"
functions, and call them manually.

Under the hood, this is done by having a specially-named tal child of
the thing we want to assist, containing the callback.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

channeld/channeld.c

@@ -2752,7 +2752,6 @@ static void handle_dev_memleak(struct peer *peer, const u8 *msg)
 
 	/* Now delete peer and things it has pointers to. */
 	memleak_remove_referenced(memtable, peer);
-	memleak_remove_htable(memtable, &peer->channel->htlcs->raw);
 
 	found_leak = dump_memleak(memtable);
 	wire_sync_write(MASTER_FD,

channeld/full_channel.c

@@ -15,6 +15,7 @@
 #include <common/htlc_wire.h>
 #include <common/key_derive.h>
 #include <common/keyset.h>
+#include <common/memleak.h>
 #include <common/status.h>
 #include <common/type_to_string.h>
 #include <inttypes.h>
@@ -23,6 +24,14 @@
   /* Needs to be at end, since it doesn't include its own hdrs */
   #include "gen_full_channel_error_names.h"
 
+#if DEVELOPER
+static void memleak_help_htlcmap(struct htable *memtable,
+				 struct htlc_map *htlcs)
+{
+	memleak_remove_htable(memtable, &htlcs->raw);
+}
+#endif /* DEVELOPER */
+
 struct channel *new_full_channel(const tal_t *ctx,
 				 const struct bitcoin_blkid *chain_hash,
 				 const struct bitcoin_txid *funding_txid,
@@ -59,6 +68,7 @@ struct channel *new_full_channel(const tal_t *ctx,
 		channel->view[REMOTE].feerate_per_kw = feerate_per_kw[REMOTE];
 		channel->htlcs = tal(channel, struct htlc_map);
 		htlc_map_init(channel->htlcs);
+		memleak_add_helper(channel->htlcs, memleak_help_htlcmap);
 		tal_add_destructor(channel->htlcs, htlc_map_clear);
 	}
 	return channel;

channeld/test/run-full_channel.c

@@ -20,6 +20,12 @@ size_t bigsize_get(const u8 *p UNNEEDED, size_t max UNNEEDED, bigsize_t *val UNN
 /* Generated stub for bigsize_put */
 size_t bigsize_put(u8 buf[BIGSIZE_MAX_LEN] UNNEEDED, bigsize_t v UNNEEDED)
 { fprintf(stderr, "bigsize_put called!\n"); abort(); }
+/* Generated stub for memleak_add_helper_ */
+void memleak_add_helper_(const tal_t *p UNNEEDED, void (*cb)(struct htable *memtable UNNEEDED,
+						    const tal_t *)){ }
+/* Generated stub for memleak_remove_htable */
+void memleak_remove_htable(struct htable *memtable UNNEEDED, const struct htable *ht UNNEEDED)
+{ fprintf(stderr, "memleak_remove_htable called!\n"); abort(); }
 /* Generated stub for status_failed */
 void status_failed(enum status_failreason code UNNEEDED,
 		   const char *fmt UNNEEDED, ...)

common/memleak.c

@@ -11,6 +11,10 @@
 static const void **notleaks;
 static bool *notleak_children;
 
+struct memleak_helper {
+	void (*cb)(struct htable *memtable, const tal_t *);
+};
+
 static size_t find_notleak(const tal_t *ptr)
 {
 	size_t i, nleaks = tal_count(notleaks);
@@ -82,6 +86,10 @@ static void children_into_htable(const void *exclude1, const void *exclude2,
 			if (streq(name, "backtrace"))
 				continue;
 
+			/* Don't add our own memleak_helpers */
+			if (strends(name, "struct memleak_helper"))
+				continue;
+
 			/* Don't add tal_link objects */
 			if (strends(name, "struct link")
 			    || strends(name, "struct linkable"))
@@ -103,20 +111,6 @@ static void children_into_htable(const void *exclude1, const void *exclude2,
 	}
 }
 
-struct htable *memleak_enter_allocations(const tal_t *ctx,
-					 const void *exclude1,
-					 const void *exclude2)
-{
-	struct htable *memtable = tal(ctx, struct htable);
-	htable_init(memtable, hash_ptr, NULL);
-
-	/* First, add all pointers off NULL to table. */
-	children_into_htable(exclude1, exclude2, memtable, NULL);
-
-	tal_add_destructor(memtable, htable_clear);
-	return memtable;
-}
-
 static void scan_for_pointers(struct htable *memtable,
 			      const tal_t *p, size_t bytelen)
 {
@@ -256,6 +250,47 @@ static void add_backtrace_notifiers(const tal_t *root)
 		add_backtrace_notifiers(i);
 }
 
+void memleak_add_helper_(const tal_t *p,
+			 void (*cb)(struct htable *memtable, const tal_t *))
+{
+	struct memleak_helper *mh = tal(p, struct memleak_helper);
+	mh->cb = cb;
+}
+
+
+static void call_memleak_helpers(struct htable *memtable, const tal_t *p)
+{
+	const tal_t *i;
+
+	for (i = tal_first(p); i; i = tal_next(i)) {
+		const char *name = tal_name(i);
+
+		if (name && strends(name, "struct memleak_helper")) {
+			const struct memleak_helper *mh = i;
+			mh->cb(memtable, p);
+		} else {
+			call_memleak_helpers(memtable, i);
+		}
+	}
+}
+
+struct htable *memleak_enter_allocations(const tal_t *ctx,
+					 const void *exclude1,
+					 const void *exclude2)
+{
+	struct htable *memtable = tal(ctx, struct htable);
+	htable_init(memtable, hash_ptr, NULL);
+
+	/* First, add all pointers off NULL to table. */
+	children_into_htable(exclude1, exclude2, memtable, NULL);
+
+	/* Iterate and call helpers to eliminate hard-to-get references. */
+	call_memleak_helpers(memtable, NULL);
+
+	tal_add_destructor(memtable, htable_clear);
+	return memtable;
+}
+
 void memleak_init(void)
 {
 	assert(!notleaks);

common/memleak.h

@@ -3,8 +3,11 @@
 #include "config.h"
 #include <ccan/cast/cast.h>
 #include <ccan/tal/tal.h>
+#include <ccan/typesafe_cb/typesafe_cb.h>
 #include <inttypes.h>
 
+struct htable;
+
 #if HAVE_TYPEOF
 #define memleak_typeof(var) typeof(var)
 #else
@@ -19,7 +22,21 @@
 
 void *notleak_(const void *ptr, bool plus_children);
 
-struct htable;
+/* Mark a helper to be called to scan this structure for mem references */
+/* For update-mock: memleak_add_helper_ mock empty */
+void memleak_add_helper_(const tal_t *p, void (*cb)(struct htable *memtable,
+						    const tal_t *));
+
+#if DEVELOPER
+#define memleak_add_helper(p, cb)					\
+	memleak_add_helper_((p),					\
+			    typesafe_cb_preargs(void, const tal_t *,	\
+						(cb), (p),		\
+						struct htable *))
+#else
+/* Don't refer to cb at all if !DEVELOPER */
+#define memleak_add_helper(p, cb)
+#endif
 
 /* Initialize memleak detection */
 void memleak_init(void);

connectd/connectd.c

@@ -1446,7 +1446,6 @@ static struct io_plan *dev_connect_memleak(struct io_conn *conn,
 
 	/* Now delete daemon and those which it has pointers to. */
 	memleak_remove_referenced(memtable, daemon);
-	memleak_remove_htable(memtable, &daemon->peers.raw);
 
 	found_leak = dump_memleak(memtable);
 	daemon_conn_send(daemon->master,
@@ -1533,6 +1532,15 @@ static void master_gone(struct daemon_conn *master UNUSED)
 	exit(2);
 }
 
+/*~ This is a hook used by the memleak code (if DEVELOPER=1): it can't see
+ * pointers inside hash tables, so we give it a hint here. */
+#if DEVELOPER
+static void memleak_daemon_cb(struct htable *memtable, struct daemon *daemon)
+{
+	memleak_remove_htable(memtable, &daemon->peers.raw);
+}
+#endif /* DEVELOPER */
+
 int main(int argc, char *argv[])
 {
 	setup_locale();
@@ -1545,6 +1553,7 @@ int main(int argc, char *argv[])
 	/* Allocate and set up our simple top-level structure. */
 	daemon = tal(NULL, struct daemon);
 	node_set_init(&daemon->peers);
+	memleak_add_helper(daemon, memleak_daemon_cb);
 	list_head_init(&daemon->connecting);
 	daemon->listen_fds = tal_arr(daemon, struct listen_fd, 0);
 	/* stdin == control */

devtools/Makefile

@@ -17,6 +17,7 @@ DEVTOOLS_COMMON_OBJS :=				\
 	common/decode_short_channel_ids.o	\
 	common/features.o			\
 	common/hash_u5.o			\
+	common/memleak.o			\
 	common/node_id.o			\
 	common/per_peer_state.o			\
 	common/json.o				\

gossipd/gossipd.c

@@ -3034,7 +3034,6 @@ static struct io_plan *dev_gossip_memleak(struct io_conn *conn,
 
 	/* Now delete daemon and those which it has pointers to. */
 	memleak_remove_referenced(memtable, daemon);
-	memleak_remove_routing_tables(memtable, daemon->rstate);
 
 	found_leak = dump_memleak(memtable);
 	daemon_conn_send(daemon->master,

gossipd/routing.c

@@ -165,6 +165,26 @@ static void destroy_routing_state(struct routing_state *rstate)
 		free_chan(rstate, chan);
 }
 
+#if DEVELOPER
+static void memleak_help_routing_tables(struct htable *memtable,
+					struct routing_state *rstate)
+{
+	struct node *n;
+	struct node_map_iter nit;
+
+	memleak_remove_htable(memtable, &rstate->nodes->raw);
+	memleak_remove_htable(memtable, &rstate->pending_node_map->raw);
+	memleak_remove_htable(memtable, &rstate->pending_cannouncements.raw);
+
+	for (n = node_map_first(rstate->nodes, &nit);
+	     n;
+	     n = node_map_next(rstate->nodes, &nit)) {
+		if (node_uses_chan_map(n))
+			memleak_remove_htable(memtable, &n->chans.map.raw);
+	}
+}
+#endif /* DEVELOPER */
+
 struct routing_state *new_routing_state(const tal_t *ctx,
 					const struct chainparams *chainparams,
 					const struct node_id *local_id,
@@ -199,6 +219,7 @@ struct routing_state *new_routing_state(const tal_t *ctx,
 		rstate->gossip_time = NULL;
 #endif
 	tal_add_destructor(rstate, destroy_routing_state);
+	memleak_add_helper(rstate, memleak_help_routing_tables);
 
 	return rstate;
 }
@@ -2529,26 +2550,6 @@ void route_prune(struct routing_state *rstate)
 	}
 }
 
-#if DEVELOPER
-void memleak_remove_routing_tables(struct htable *memtable,
-				   const struct routing_state *rstate)
-{
-	struct node *n;
-	struct node_map_iter nit;
-
-	memleak_remove_htable(memtable, &rstate->nodes->raw);
-	memleak_remove_htable(memtable, &rstate->pending_node_map->raw);
-	memleak_remove_htable(memtable, &rstate->pending_cannouncements.raw);
-
-	for (n = node_map_first(rstate->nodes, &nit);
-	     n;
-	     n = node_map_next(rstate->nodes, &nit)) {
-		if (node_uses_chan_map(n))
-			memleak_remove_htable(memtable, &n->chans.map.raw);
-	}
-}
-#endif /* DEVELOPER */
-
 bool handle_local_add_channel(struct routing_state *rstate,
 			      const u8 *msg, u64 index)
 {

gossipd/routing.h

@@ -396,9 +396,6 @@ bool routing_add_node_announcement(struct routing_state *rstate,
 bool handle_local_add_channel(struct routing_state *rstate, const u8 *msg,
 			      u64 index);
 
-void memleak_remove_routing_tables(struct htable *memtable,
-				   const struct routing_state *rstate);
-
 /**
  * Get the local time.
  *

gossipd/test/run-bench-find_route.c

@@ -38,6 +38,9 @@ bool fromwire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const void
 /* Generated stub for fromwire_wireaddr */
 bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED)
 { fprintf(stderr, "fromwire_wireaddr called!\n"); abort(); }
+/* Generated stub for memleak_add_helper_ */
+void memleak_add_helper_(const tal_t *p UNNEEDED, void (*cb)(struct htable *memtable UNNEEDED,
+						    const tal_t *)){ }
 /* Generated stub for onion_type_name */
 const char *onion_type_name(int e UNNEEDED)
 { fprintf(stderr, "onion_type_name called!\n"); abort(); }

gossipd/test/run-crc32_of_update.c

@@ -197,10 +197,6 @@ struct htable *memleak_enter_allocations(const tal_t *ctx UNNEEDED,
 /* 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_remove_routing_tables */
-void memleak_remove_routing_tables(struct htable *memtable UNNEEDED,
-				   const struct routing_state *rstate UNNEEDED)
-{ fprintf(stderr, "memleak_remove_routing_tables called!\n"); abort(); }
 /* Generated stub for new_reltimer_ */
 struct oneshot *new_reltimer_(struct timers *timers UNNEEDED,
 			      const tal_t *ctx UNNEEDED,

gossipd/test/run-extended-info.c

@@ -220,10 +220,6 @@ struct htable *memleak_enter_allocations(const tal_t *ctx UNNEEDED,
 /* 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_remove_routing_tables */
-void memleak_remove_routing_tables(struct htable *memtable UNNEEDED,
-				   const struct routing_state *rstate UNNEEDED)
-{ fprintf(stderr, "memleak_remove_routing_tables called!\n"); abort(); }
 /* Generated stub for new_reltimer_ */
 struct oneshot *new_reltimer_(struct timers *timers UNNEEDED,
 			      const tal_t *ctx UNNEEDED,

gossipd/test/run-find_route-specific.c

@@ -27,6 +27,9 @@ bool fromwire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const void
 /* Generated stub for fromwire_wireaddr */
 bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED)
 { fprintf(stderr, "fromwire_wireaddr called!\n"); abort(); }
+/* Generated stub for memleak_add_helper_ */
+void memleak_add_helper_(const tal_t *p UNNEEDED, void (*cb)(struct htable *memtable UNNEEDED,
+						    const tal_t *)){ }
 /* Generated stub for onion_type_name */
 const char *onion_type_name(int e UNNEEDED)
 { fprintf(stderr, "onion_type_name called!\n"); abort(); }

gossipd/test/run-find_route.c

@@ -25,6 +25,9 @@ bool fromwire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const void
 /* Generated stub for fromwire_wireaddr */
 bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED)
 { fprintf(stderr, "fromwire_wireaddr called!\n"); abort(); }
+/* Generated stub for memleak_add_helper_ */
+void memleak_add_helper_(const tal_t *p UNNEEDED, void (*cb)(struct htable *memtable UNNEEDED,
+						    const tal_t *)){ }
 /* Generated stub for onion_type_name */
 const char *onion_type_name(int e UNNEEDED)
 { fprintf(stderr, "onion_type_name called!\n"); abort(); }

gossipd/test/run-overlong.c

@@ -25,6 +25,9 @@ bool fromwire_gossip_store_private_update(const tal_t *ctx UNNEEDED, const void
 /* Generated stub for fromwire_wireaddr */
 bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED)
 { fprintf(stderr, "fromwire_wireaddr called!\n"); abort(); }
+/* Generated stub for memleak_add_helper_ */
+void memleak_add_helper_(const tal_t *p UNNEEDED, void (*cb)(struct htable *memtable UNNEEDED,
+						    const tal_t *)){ }
 /* Generated stub for onion_type_name */
 const char *onion_type_name(int e UNNEEDED)
 { fprintf(stderr, "onion_type_name called!\n"); abort(); }

lightningd/jsonrpc.c

@@ -877,6 +877,14 @@ static void destroy_jsonrpc(struct jsonrpc *jsonrpc)
 	strmap_clear(&jsonrpc->usagemap);
 }
 
+#if DEVELOPER
+static void memleak_help_jsonrpc(struct htable *memtable,
+				 struct jsonrpc *jsonrpc)
+{
+	memleak_remove_strmap(memtable, &jsonrpc->usagemap);
+}
+#endif /* DEVELOPER */
+
 void jsonrpc_setup(struct lightningd *ld)
 {
 	struct json_command **commands = get_cmdlist();
@@ -893,6 +901,7 @@ void jsonrpc_setup(struct lightningd *ld)
 	}
 	ld->jsonrpc->rpc_listener = NULL;
 	tal_add_destructor(ld->jsonrpc, destroy_jsonrpc);
+	memleak_add_helper(ld->jsonrpc, memleak_help_jsonrpc);
 }
 
 bool command_usage_only(const struct command *cmd)
@@ -1201,11 +1210,3 @@ static const struct json_command check_command = {
 };
 
 AUTODATA(json_command, &check_command);
-
-#if DEVELOPER
-void jsonrpc_remove_memleak(struct htable *memtable,
-			    const struct jsonrpc *jsonrpc)
-{
-	memleak_remove_strmap(memtable, &jsonrpc->usagemap);
-}
-#endif /* DEVELOPER */

lightningd/jsonrpc.h

@@ -211,12 +211,4 @@ void jsonrpc_request_end(struct jsonrpc_request *request);
 
 AUTODATA_TYPE(json_command, struct json_command);
 
-#if DEVELOPER
-struct htable;
-struct jsonrpc;
-
-void jsonrpc_remove_memleak(struct htable *memtable,
-			    const struct jsonrpc *jsonrpc);
-#endif /* DEVELOPER */
-
 #endif /* LIGHTNING_LIGHTNINGD_JSONRPC_H */

lightningd/memdump.c

@@ -161,7 +161,6 @@ static void scan_mem(struct command *cmd,
 	memleak_remove_htable(memtable, &ld->topology->txowatches.raw);
 	memleak_remove_htable(memtable, &ld->htlcs_in.raw);
 	memleak_remove_htable(memtable, &ld->htlcs_out.raw);
-	jsonrpc_remove_memleak(memtable, ld->jsonrpc);
 
 	/* Now delete ld and those which it has pointers to. */
 	memleak_remove_referenced(memtable, ld);

wallet/test/run-wallet.c

@@ -540,7 +540,7 @@ u8 *towire_channel_dev_memleak(const tal_t *ctx UNNEEDED)
 u8 *towire_channel_dev_reenable_commit(const tal_t *ctx UNNEEDED)
 { fprintf(stderr, "towire_channel_dev_reenable_commit called!\n"); abort(); }
 /* Generated stub for towire_channel_fail_htlc */
-u8 *towire_channel_fail_htlc(const tal_t *ctx UNNEEDED, const struct failed_htlc *failed_htlc UNNEEDED, u32 failheight UNUSED)
+u8 *towire_channel_fail_htlc(const tal_t *ctx UNNEEDED, const struct failed_htlc *failed_htlc UNNEEDED, u32 failheight UNNEEDED)
 { fprintf(stderr, "towire_channel_fail_htlc called!\n"); abort(); }
 /* Generated stub for towire_channel_fulfill_htlc */
 u8 *towire_channel_fulfill_htlc(const tal_t *ctx UNNEEDED, const struct fulfilled_htlc *fulfilled_htlc UNNEEDED)