common: add new_htable() macro to allocate, initialize and setup memleak coverage for any typed hash table.

You can now simply add per-tal-object helpers for memleak, but our older pattern required
calling memleak functions explicitly during memleak handling.  Hash tables in particular need
to be dynamically allocated (we override the allocators using htable_set_allocator and assume
this), so it makes sense to have a helper macro that does all three.

This eliminates a huge amount of code.

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

Author: rustyrussell

channeld/full_channel.c

@@ -18,12 +18,6 @@
   /* Needs to be at end, since it doesn't include its own hdrs */
   #include "full_channel_error_names_gen.h"
 
-static void memleak_help_htlcmap(struct htable *memtable,
-				 struct htlc_map *htlcs)
-{
-	memleak_scan_htable(memtable, &htlcs->raw);
-}
-
 /* This is a dangerous thing!  Because we apply HTLCs in many places
  * in bulk, we can temporarily go negative.  You must check balance_ok()
  * at the end! */
@@ -114,11 +108,9 @@ struct channel *new_full_channel(const tal_t *ctx,
 						      option_wumbo,
 						      opener);
 
-	if (channel) {
-		channel->htlcs = tal(channel, struct htlc_map);
-		htlc_map_init(channel->htlcs);
-		memleak_add_helper(channel->htlcs, memleak_help_htlcmap);
-	}
+	if (channel)
+		channel->htlcs = new_htable(channel, htlc_map);
+
 	return channel;
 }
 

common/memleak.h

@@ -107,6 +107,18 @@ void memleak_scan_region(struct htable *memtable, const void *p, size_t len);
 /* Objects inside this htable (which is opaque to memleak) are not leaks. */
 void memleak_scan_htable(struct htable *memtable, const struct htable *ht);
 
+/* Allocate a htable, set up memleak scan automatically (assumes &p->raw == p) */
+#define new_htable(ctx, type)				\
+	({							  \
+		const struct htable *raw;			  \
+		struct type *p = tal(ctx, struct type);		  \
+		type##_init(p);					  \
+		raw = &p->raw;					  \
+		assert((void *)raw == (void *)p);		  \
+		memleak_add_helper(raw, memleak_scan_htable);	  \
+		p;						  \
+	})
+
 /* Objects inside this uintmap (which is opaque to memleak) are not leaks. */
 #define memleak_scan_uintmap(memtable, umap)		\
 	memleak_scan_intmap_(memtable, uintmap_unwrap_(umap))

connectd/connectd.c

@@ -2019,9 +2019,6 @@ static void dev_connect_memleak(struct daemon *daemon, const u8 *msg)
 
 	/* Now delete daemon and those which it has pointers to. */
 	memleak_scan_obj(memtable, daemon);
-	memleak_scan_htable(memtable, &daemon->peers->raw);
-	memleak_scan_htable(memtable, &daemon->scid_htable->raw);
-	memleak_scan_htable(memtable, &daemon->important_ids->raw);
 
 	found_leak = dump_memleak(memtable, memleak_status_broken, NULL);
 	daemon_conn_send(daemon->master,
@@ -2435,14 +2432,6 @@ static struct io_plan *recv_gossip(struct io_conn *conn,
 	return daemon_conn_read_next(conn, daemon->gossipd);
 }
 
-/*~ This is a hook used by the memleak code: it can't see pointers
- * inside hash tables, so we give it a hint here. */
-static void memleak_daemon_cb(struct htable *memtable, struct daemon *daemon)
-{
-	memleak_scan_htable(memtable, &daemon->peers->raw);
-	memleak_scan_htable(memtable, &daemon->connecting->raw);
-}
-
 static void gossipd_failed(struct daemon_conn *gossipd)
 {
 	status_failed(STATUS_FAIL_GOSSIP_IO, "gossipd exited?");
@@ -2462,14 +2451,13 @@ int main(int argc, char *argv[])
 	daemon = tal(NULL, struct daemon);
 	daemon->developer = developer;
 	daemon->connection_counter = 1;
-	daemon->peers = tal(daemon, struct peer_htable);
+	/* htable_new is our helper which allocates a htable, initializes it
+	 * and set up the memleak callback so our memleak code can see objects
+	 * inside it */
+	daemon->peers = new_htable(daemon, peer_htable);
 	daemon->listeners = tal_arr(daemon, struct io_listener *, 0);
-	peer_htable_init(daemon->peers);
-	memleak_add_helper(daemon, memleak_daemon_cb);
-	daemon->connecting = tal(daemon, struct connecting_htable);
-	connecting_htable_init(daemon->connecting);
-	daemon->important_ids = tal(daemon, struct important_id_htable);
-	important_id_htable_init(daemon->important_ids);
+	daemon->connecting = new_htable(daemon, connecting_htable);
+	daemon->important_ids = new_htable(daemon, important_id_htable);
 	timers_init(&daemon->timers, time_mono());
 	daemon->gossmap_raw = NULL;
 	daemon->shutting_down = false;
@@ -2479,8 +2467,7 @@ int main(int argc, char *argv[])
 	daemon->dev_exhausted_fds = false;
 	/* We generally allow 1MB per second per peer, except for dev testing */
 	daemon->gossip_stream_limit = 1000000;
-	daemon->scid_htable = tal(daemon, struct scid_htable);
-	scid_htable_init(daemon->scid_htable);
+	daemon->scid_htable = new_htable(daemon, scid_htable);
 
 	/* stdin == control */
 	daemon->master = daemon_conn_new(daemon, STDIN_FILENO, recv_req, NULL,

gossipd/gossipd.c

@@ -472,7 +472,6 @@ static void dev_gossip_memleak(struct daemon *daemon, const u8 *msg)
 	memleak_ptr(memtable, msg);
 	/* Now delete daemon and those which it has pointers to. */
 	memleak_scan_obj(memtable, daemon);
-	memleak_scan_htable(memtable, &daemon->peers->raw);
 	dev_seeker_memleak(memtable, daemon->seeker);
 	gossmap_manage_memleak(memtable, daemon->gm);
 
@@ -625,8 +624,7 @@ int main(int argc, char *argv[])
 	daemon = tal(NULL, struct daemon);
 	daemon->developer = developer;
 	daemon->dev_gossip_time = NULL;
-	daemon->peers = tal(daemon, struct peer_node_id_map);
-	peer_node_id_map_init(daemon->peers);
+	daemon->peers = new_htable(daemon, peer_node_id_map);
 	daemon->deferred_txouts = tal_arr(daemon, struct short_channel_id, 0);
 	daemon->current_blockheight = 0; /* i.e. unknown */
 

lightningd/chaintopology.c

@@ -1220,14 +1220,10 @@ struct chain_topology *new_topology(struct lightningd *ld, struct logger *log)
 	struct chain_topology *topo = tal(ld, struct chain_topology);
 
 	topo->ld = ld;
-	topo->block_map = tal(topo, struct block_map);
-	block_map_init(topo->block_map);
-	topo->outgoing_txs = tal(topo, struct outgoing_tx_map);
-	outgoing_tx_map_init(topo->outgoing_txs);
-	topo->txwatches = tal(topo, struct txwatch_hash);
-	txwatch_hash_init(topo->txwatches);
-	topo->txowatches = tal(topo, struct txowatch_hash);
-	txowatch_hash_init(topo->txowatches);
+	topo->block_map = new_htable(topo, block_map);
+	topo->outgoing_txs = new_htable(topo, outgoing_tx_map);
+	topo->txwatches = new_htable(topo, txwatch_hash);
+	topo->txowatches = new_htable(topo, txowatch_hash);
 	topo->log = log;
 	topo->bitcoind = new_bitcoind(topo, ld, log);
 	topo->poll_seconds = 30;

lightningd/lightningd.c

@@ -186,42 +186,34 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 	 * list attached to the channel structure itself, or even left them in
 	 * the database rather than making an in-memory version.  Obviously
 	 * I was in a premature optimization mood when I wrote this: */
-	ld->htlcs_in = tal(ld, struct htlc_in_map);
-	htlc_in_map_init(ld->htlcs_in);
+	ld->htlcs_in = new_htable(ld, htlc_in_map);
 
 	/*~ Note also: we didn't need to use an allocation here!  We could
 	 * have simply made the `struct htlc_out_map` a member.  But we
 	 * override the htable allocation routines to use tal(), and they
 	 * want a tal parent, so we always make our hash table a tallocated
 	 * object. */
-	ld->htlcs_out = tal(ld, struct htlc_out_map);
-	htlc_out_map_init(ld->htlcs_out);
+	ld->htlcs_out = new_htable(ld, htlc_out_map);
 
 	/*~ This is the hash table of peers: converted from a
 	 *  linked-list as part of the 100k-peers project! */
-	ld->peers = tal(ld, struct peer_node_id_map);
-	peer_node_id_map_init(ld->peers);
+	ld->peers = new_htable(ld, peer_node_id_map);
 	/*~ And this was done at the same time, for db lookups at startup */
-	ld->peers_by_dbid = tal(ld, struct peer_dbid_map);
-	peer_dbid_map_init(ld->peers_by_dbid);
+	ld->peers_by_dbid = new_htable(ld, peer_dbid_map);
 
 	/*~ This speeds lookups for short_channel_ids to their channels. */
-	ld->channels_by_scid = tal(ld, struct channel_scid_map);
-	channel_scid_map_init(ld->channels_by_scid);
+	ld->channels_by_scid = new_htable(ld, channel_scid_map);
 
 	/*~ Coin movements in db are indexed by the channel dbid. */
-	ld->channels_by_dbid = tal(ld, struct channel_dbid_map);
-	channel_dbid_map_init(ld->channels_by_dbid);
+	ld->channels_by_dbid = new_htable(ld, channel_dbid_map);
 
 	/*~ For multi-part payments, we need to keep some incoming payments
 	 * in limbo until we get all the parts, or we time them out. */
-	ld->htlc_sets = tal(ld, struct htlc_set_map);
-	htlc_set_map_init(ld->htlc_sets);
+	ld->htlc_sets = new_htable(ld, htlc_set_map);
 
 	/*~ We keep a map of closed channels.  Mainly so we can respond to peers
 	 * who talk to us about long-closed channels. */
-	ld->closed_channels = tal(ld, struct closed_channel_map);
-	closed_channel_map_init(ld->closed_channels);
+	ld->closed_channels = new_htable(ld, closed_channel_map);
 
 	/*~ We have a multi-entry log-book infrastructure: we define a 10MB log
 	 * book to hold all the entries (and trims as necessary), and multiple

lightningd/memdump.c

@@ -189,19 +189,6 @@ static bool lightningd_check_leaks(struct command *cmd)
 	memleak_ptr(memtable, cmd);
 	memleak_ignore_children(memtable, cmd);
 
-	/* First delete known false positives. */
-	memleak_scan_htable(memtable, &ld->topology->txwatches->raw);
-	memleak_scan_htable(memtable, &ld->topology->txowatches->raw);
-	memleak_scan_htable(memtable, &ld->topology->outgoing_txs->raw);
-	memleak_scan_htable(memtable, &ld->htlcs_in->raw);
-	memleak_scan_htable(memtable, &ld->htlcs_out->raw);
-	memleak_scan_htable(memtable, &ld->htlc_sets->raw);
-	memleak_scan_htable(memtable, &ld->peers->raw);
-	memleak_scan_htable(memtable, &ld->peers_by_dbid->raw);
-	memleak_scan_htable(memtable, &ld->channels_by_scid->raw);
-	memleak_scan_htable(memtable, &ld->closed_channels->raw);
-	wallet_memleak_scan(memtable, ld->wallet);
-
 	/* Now delete ld and those which it has pointers to. */
 	memleak_scan_obj(memtable, ld);
 

lightningd/onchain_control.c

@@ -43,13 +43,6 @@ static bool replay_tx_eq_txid(const struct replay_tx *rtx,
 HTABLE_DEFINE_NODUPS_TYPE(struct replay_tx, replay_tx_keyof, txid_hash, replay_tx_eq_txid,
 			  replay_tx_hash);
 
-/* Helper for memleak detection */
-static void memleak_replay_tx_hash(struct htable *memtable,
-				   struct replay_tx_hash *replay_tx_hash)
-{
-	memleak_scan_htable(memtable, &replay_tx_hash->raw);
-}
-
 /* We dump all the known preimages when onchaind starts up. */
 static void onchaind_tell_fulfill(struct channel *channel)
 {
@@ -1898,11 +1891,8 @@ void onchaind_replay_channels(struct lightningd *ld)
 				 channel_state_name(channel), blockheight);
 
 			/* We're in replay mode */
-			channel->onchaind_replay_watches = tal(channel, struct replay_tx_hash);
+			channel->onchaind_replay_watches = new_htable(channel, replay_tx_hash);
 			channel->onchaind_replay_height = blockheight;
-			replay_tx_hash_init(channel->onchaind_replay_watches);
-			memleak_add_helper(channel->onchaind_replay_watches,
-					   memleak_replay_tx_hash);
 
 			onchaind_funding_spent(channel, tx, blockheight);
 			onchaind_replay(channel);

plugins/askrene/askrene.c

@@ -1291,13 +1291,6 @@ static const struct plugin_command commands[] = {
 	},
 };
 
-static void askrene_markmem(struct plugin *plugin, struct htable *memtable)
-{
-	struct askrene *askrene = get_askrene(plugin);
-	layer_memleak_mark(askrene, memtable);
-	reserve_memleak_mark(askrene, memtable);
-}
-
 static const char *init(struct command *init_cmd,
 			const char *buf UNUSED, const jsmntok_t *config UNUSED)
 {
@@ -1317,7 +1310,6 @@ static const char *init(struct command *init_cmd,
 		 "{id:%}", JSON_SCAN(json_to_node_id, &askrene->my_id));
 
 	plugin_set_data(plugin, askrene);
-	plugin_set_memleak_handler(plugin, askrene_markmem);
 
 	load_layers(askrene, init_cmd);
 

plugins/askrene/layer.c

@@ -162,14 +162,10 @@ struct layer *new_temp_layer(const tal_t *ctx, struct askrene *askrene, const ch
 	l->askrene = askrene;
 	l->name = tal_strdup(l, name);
 	l->persistent = false;
-	l->local_channels = tal(l, struct local_channel_hash);
-	local_channel_hash_init(l->local_channels);
-	l->local_updates = tal(l, struct local_update_hash);
-	local_update_hash_init(l->local_updates);
-	l->constraints = tal(l, struct constraint_hash);
-	constraint_hash_init(l->constraints);
-	l->biases = tal(l, struct bias_hash);
-	bias_hash_init(l->biases);
+	l->local_channels = new_htable(l, local_channel_hash);
+	l->local_updates = new_htable(l, local_update_hash);
+	l->constraints = new_htable(l, constraint_hash);
+	l->biases = new_htable(l, bias_hash);
 	l->disabled_nodes = tal_arr(l, struct node_id, 0);
 
 	return l;
@@ -1162,14 +1158,3 @@ bool layer_disables_node(const struct layer *layer,
 	}
 	return false;
 }
-
-void layer_memleak_mark(struct askrene *askrene, struct htable *memtable)
-{
-	struct layer *l;
-	list_for_each(&askrene->layers, l, list) {
-		memleak_scan_htable(memtable, &l->constraints->raw);
-		memleak_scan_htable(memtable, &l->local_channels->raw);
-		memleak_scan_htable(memtable, &l->local_updates->raw);
-		memleak_scan_htable(memtable, &l->biases->raw);
-	}
-}