utils: add a global tmpctx.

I did a brief audit of tmpctx uses, and we do leak them in various
corner cases.  Fortunely, all our daemons are based on some kind of
I/O loop, so it's fairly easy to clean a global tmpctx at that point.

This makes things a bit neater, and slightly more efficient, but also
clearer: I avoided creating a tmpctx in a few places because I didn't
want to add another allocation.  With that penalty removed, I can use
it more freely and hopefully write clearer code.

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

channeld/channel.c

@@ -1856,7 +1856,8 @@ static void peer_reconnect(struct peer *peer)
 	peer_billboard(false, "Sent reestablish, waiting for theirs");
 
 	/* Read until they say something interesting */
-	while ((msg = channeld_read_peer_msg(peer)) == NULL);
+	while ((msg = channeld_read_peer_msg(peer)) == NULL)
+		clean_tmpctx();
 
 	if (!fromwire_channel_reestablish(msg, &channel_id,
 					  &next_local_commitment_number,
@@ -2698,6 +2699,9 @@ int main(int argc, char *argv[])
 		const u8 *msg;
 		struct timemono now = time_mono();
 
+		/* Free any temporary allocations */
+		clean_tmpctx();
+
 		/* For simplicity, we process one event at a time. */
 		msg = msg_dequeue(&peer->from_master);
 		if (msg) {
@@ -2788,6 +2792,6 @@ int main(int argc, char *argv[])
 	/* We only exit when shutdown is complete. */
 	assert(shutdown_complete(peer));
 	send_shutdown_complete(peer);
-
+	tal_free(tmpctx);
 	return 0;
 }

closingd/closing.c

@@ -591,6 +591,7 @@ int main(int argc, char *argv[])
 	wire_sync_write(REQ_FD,
 			take(towire_closing_complete(ctx, gossip_index)));
 	tal_free(ctx);
+	tal_free(tmpctx);
 
 	return 0;
 }

common/io_debug.c

@@ -11,9 +11,7 @@ int debug_poll(struct pollfd *fds, nfds_t nfds, int timeout)
 	if (t)
 		errx(1, "Outstanding taken pointers: %s", t);
 
-	t = tmpctx_any();
-	if (t)
-		errx(1, "Outstanding tmpctx: %s", t);
+	clean_tmpctx();
 
 	return poll(fds, nfds, timeout);
 }

common/subdaemon.c

@@ -73,6 +73,8 @@ void subdaemon_setup(int argc, char *argv[])
 	secp256k1_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY
 						 | SECP256K1_CONTEXT_SIGN);
 
+	setup_tmpctx();
+
 	for (int i = 1; i < argc; i++) {
 		if (streq(argv[i], "--log-io"))
 			logging_io = true;

common/utils.c

@@ -4,6 +4,7 @@
 #include <ccan/tal/str/str.h>
 
 secp256k1_context *secp256k1_ctx;
+const tal_t *tmpctx;
 
 char *tal_hexstr(const tal_t *ctx, const void *data, size_t len)
 {
@@ -25,36 +26,20 @@ u8 *tal_hexdata(const tal_t *ctx, const void *str, size_t len)
 	return data;
 }
 
-struct tmpctx {
-	struct list_node list;
-	const char *file;
-	unsigned int line;
-};
+/* Global temporary convenience context: freed in io loop core. */
 
-static struct list_head tmpctxs = LIST_HEAD_INIT(tmpctxs);
-
-static void destroy_tmpctx(struct tmpctx *t)
+/* Initial creation of tmpctx. */
+void setup_tmpctx(void)
 {
-	list_del_from(&tmpctxs, &t->list);
+	tmpctx = tal(NULL, char);
 }
 
-tal_t *tal_tmpctx_(const tal_t *ctx, const char *file, unsigned int line)
+/* Free any children of tmpctx. */
+void clean_tmpctx(void)
 {
-	struct tmpctx *t = tal(ctx, struct tmpctx);
-	t->file = file;
-	t->line = line;
-	list_add_tail(&tmpctxs, &t->list);
-	tal_add_destructor(t, destroy_tmpctx);
-	return t;
-}
-
-const char *tmpctx_any(void)
-{
-	struct tmpctx *t = list_top(&tmpctxs, struct tmpctx, list);
-
-	if (t) {
-		assert(t->file != NULL);
-		return tal_fmt(t, "%s:%u", t->file, t->line);
+	/* Minor optimization: don't do anything if tmpctx unused. */
+	if (tal_first(tmpctx)) {
+		tal_free(tmpctx);
+		tmpctx = tal(NULL, char);
 	}
-	return NULL;
 }

common/utils.h

@@ -16,12 +16,16 @@ char *tal_hex(const tal_t *ctx, const tal_t *data);
 /* Allocate and fill a buffer with the data of this hex string. */
 u8 *tal_hexdata(const tal_t *ctx, const void *str, size_t len);
 
-/* Get a temporary context for this function scope (tal_free at end) */
-tal_t *tal_tmpctx_(const tal_t *ctx, const char *file, unsigned int line);
-#define tal_tmpctx(ctx)							\
-	tal_tmpctx_((ctx), __FILE__, __LINE__)
+/* FIXME: Remove in favor of global */
+#define tal_tmpctx(ctx) tal((ctx), char)
 
-/* Return non-NULL if any tmpctx still allocated. */
-const char *tmpctx_any(void);
+/* Global temporary convenience context: freed in io loop core. */
+extern const tal_t *tmpctx;
+
+/* Initial creation of tmpctx. */
+void setup_tmpctx(void);
+
+/* Free any children of tmpctx. */
+void clean_tmpctx(void);
 
 #endif /* LIGHTNING_COMMON_UTILS_H */

lightningd/lightningd.c

@@ -296,6 +296,7 @@ int main(int argc, char *argv[])
 	ld = new_lightningd(NULL);
 	secp256k1_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY
 						 | SECP256K1_CONTEXT_SIGN);
+	setup_tmpctx();
 
 	io_poll_override(debug_poll);
 

lightningd/test/run-key_derive.c

@@ -1,5 +1,4 @@
 #define SUPERVERBOSE
-static void *tmpctx;
 
 #include <assert.h>
 #include <ccan/str/hex/hex.h>
@@ -23,7 +22,7 @@ int main(void)
 	struct secret base_secret, per_commitment_secret;
 	struct pubkey base_point, per_commitment_point, pubkey, pubkey2;
 
-	tmpctx = tal_tmpctx(NULL);
+	setup_tmpctx();
 	secp256k1_ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY
 						 | SECP256K1_CONTEXT_SIGN);
 

onchaind/onchain.c

@@ -1105,6 +1105,7 @@ static void wait_for_resolved(struct tracked_output **outs)
 
 		billboard_update(outs);
 		tal_free(msg);
+		clean_tmpctx();
 	}
 
 	wire_sync_write(REQ_FD,
@@ -2315,6 +2316,7 @@ int main(int argc, char *argv[])
 
 	/* We're done! */
 	tal_free(ctx);
+	tal_free(tmpctx);
 
 	return 0;
 }

openingd/opening.c

@@ -195,7 +195,8 @@ static u8 *opening_read_peer_msg(struct state *state)
 				    &state->channel_id,
 				    sync_crypto_write_arg,
 				    status_fail_io,
-				    state)) == NULL);
+				    state)) == NULL)
+		clean_tmpctx();
 
 	return msg;
 }
@@ -772,6 +773,7 @@ int main(int argc, char *argv[])
 	status_trace("Sent %s with fd",
 		     opening_wire_type_name(fromwire_peektype(msg)));
 	tal_free(state);
+	tal_free(tmpctx);
 	return 0;
 }
 #endif /* TESTING */