From 1430036684da97eec607abb7d43d55842baf8fd9 Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Thu, 22 Nov 2018 12:47:29 +1030
Subject: [PATCH] connectd: wire up dev_memleak.
We need several notleak() annotations here:
1. The temporary structure which is handed to retry_peer_connected().
It's waiting for the master to respond to our connect_reconnected
message.
2. We don't keep a pointer to the io_conn for a peer, so we need to
mark those as not being a leak.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
connectd/connect_wire.csv | 6 ++++
connectd/connectd.c | 55 +++++++++++++++++++++++++++++++-----
lightningd/connect_control.c | 2 ++
lightningd/memdump.c | 29 +++++++++++++++++--
4 files changed, 83 insertions(+), 9 deletions(-)
diff --git a/connectd/connect_wire.csv b/connectd/connect_wire.csv
index 2fcfdb657..49463ea54 100644
--- a/connectd/connect_wire.csv
+++ b/connectd/connect_wire.csv
@@ -62,3 +62,9 @@ connect_peer_connected,,localfeatures,lflen*u8
# master -> connectd: peer has disconnected.
connectctl_peer_disconnected,2015
connectctl_peer_disconnected,,id,struct pubkey
+
+# master -> connectd: do you have a memleak?
+connect_dev_memleak,2033
+
+connect_dev_memleak_reply,2133
+connect_dev_memleak_reply,,leak,bool
diff --git a/connectd/connectd.c b/connectd/connectd.c
index f89f6e0a3..7745b98d3 100644
--- a/connectd/connectd.c
+++ b/connectd/connectd.c
@@ -29,6 +29,7 @@
#include <common/daemon_conn.h>
#include <common/decode_short_channel_ids.h>
#include <common/features.h>
+#include <common/memleak.h>
#include <common/ping.h>
#include <common/pseudorand.h>
#include <common/status.h>
@@ -392,7 +393,11 @@ static struct io_plan *peer_reconnected(struct io_conn *conn,
* the peer set. When someone calls `io_wake()` on that address, it
* will call retry_peer_connected above. */
return io_wait(conn, pubkey_set_get(&daemon->peers, id),
- retry_peer_connected, pr);
+ /*~ The notleak() wrapper is a DEVELOPER-mode hack so
+ * that our memory leak detection doesn't consider 'pr'
+ * (which is not referenced from our code) to be a
+ * memory leak. */
+ retry_peer_connected, notleak(pr));
}
/*~ Note the lack of static: this is called by peer_exchange_initmsg.c once the
@@ -490,8 +495,11 @@ static struct io_plan *connection_in(struct io_conn *conn, struct daemon *daemon
/* FIXME: Timeout */
/*~ The crypto handshake differs depending on whether you received or
- * initiated the socket connection, so there are two entry points. */
- return responder_handshake(conn, &daemon->id, &addr,
+ * initiated the socket connection, so there are two entry points.
+ * Note, again, the notleak() to avoid our simplistic leak detection
+ * code from thinking `conn` (which we don't keep a pointer to) is
+ * leaked */
+ return responder_handshake(notleak(conn), &daemon->id, &addr,
handshake_in_success, daemon);
}
@@ -738,9 +746,9 @@ static void try_connect_one_addr(struct connecting *connect)
/* This creates the new connection using our fd, with the initialization
* function one of the above. */
if (use_proxy)
- io_new_conn(connect, fd, conn_proxy_init, connect);
+ notleak(io_new_conn(connect, fd, conn_proxy_init, connect));
else
- io_new_conn(connect, fd, conn_init, connect);
+ notleak(io_new_conn(connect, fd, conn_init, connect));
}
/*~ connectd is responsible for incoming connections, but it's the process of
@@ -1127,6 +1135,11 @@ static struct io_plan *connect_init(struct io_conn *conn,
tor_password,
&announcable);
+ /* Free up old allocations */
+ tal_free(proposed_wireaddr);
+ tal_free(proposed_listen_announce);
+ tal_free(tor_password);
+
/* Tell it we're ready, handing it the addresses we have. */
daemon_conn_send(daemon->master,
take(towire_connectctl_init_reply(NULL,
@@ -1159,8 +1172,9 @@ static struct io_plan *connect_activate(struct io_conn *conn,
"Failed to listen on socket: %s",
strerror(errno));
}
- io_new_listener(daemon, daemon->listen_fds[i].fd,
- connection_in, daemon);
+ notleak(io_new_listener(daemon,
+ daemon->listen_fds[i].fd,
+ connection_in, daemon));
}
}
/* Free, with NULL assignment just as an extra sanity check. */
@@ -1366,6 +1380,28 @@ static struct io_plan *peer_disconnected(struct io_conn *conn,
return daemon_conn_read_next(conn, daemon->master);
}
+#if DEVELOPER
+static struct io_plan *dev_connect_memleak(struct io_conn *conn,
+ struct daemon *daemon,
+ const u8 *msg)
+{
+ struct htable *memtable;
+ bool found_leak;
+
+ memtable = memleak_enter_allocations(tmpctx, msg, msg);
+
+ /* 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,
+ take(towire_connect_dev_memleak_reply(NULL,
+ found_leak)));
+ return daemon_conn_read_next(conn, daemon->master);
+}
+#endif /* DEVELOPER */
+
static struct io_plan *recv_req(struct io_conn *conn,
const u8 *msg,
struct daemon *daemon)
@@ -1387,12 +1423,17 @@ static struct io_plan *recv_req(struct io_conn *conn,
case WIRE_CONNECTCTL_PEER_DISCONNECTED:
return peer_disconnected(conn, daemon, msg);
+ case WIRE_CONNECT_DEV_MEMLEAK:
+#if DEVELOPER
+ return dev_connect_memleak(conn, daemon, msg);
+#endif
/* We send these, we don't receive them */
case WIRE_CONNECTCTL_INIT_REPLY:
case WIRE_CONNECTCTL_ACTIVATE_REPLY:
case WIRE_CONNECT_PEER_CONNECTED:
case WIRE_CONNECT_RECONNECTED:
case WIRE_CONNECTCTL_CONNECT_FAILED:
+ case WIRE_CONNECT_DEV_MEMLEAK_REPLY:
break;
}
diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c
index e9db5ce9f..8fc0220b1 100644
--- a/lightningd/connect_control.c
+++ b/lightningd/connect_control.c
@@ -291,9 +291,11 @@ static unsigned connectd_msg(struct subd *connectd, const u8 *msg, const int *fd
case WIRE_CONNECTCTL_ACTIVATE:
case WIRE_CONNECTCTL_CONNECT_TO_PEER:
case WIRE_CONNECTCTL_PEER_DISCONNECTED:
+ case WIRE_CONNECT_DEV_MEMLEAK:
/* This is a reply, so never gets through to here. */
case WIRE_CONNECTCTL_INIT_REPLY:
case WIRE_CONNECTCTL_ACTIVATE_REPLY:
+ case WIRE_CONNECT_DEV_MEMLEAK_REPLY:
break;
case WIRE_CONNECT_RECONNECTED:
diff --git a/lightningd/memdump.c b/lightningd/memdump.c
index 2a74b4797..2f484fcc0 100644
--- a/lightningd/memdump.c
+++ b/lightningd/memdump.c
@@ -6,6 +6,7 @@
#include <common/daemon.h>
#include <common/memleak.h>
#include <common/timeout.h>
+#include <connectd/gen_connect_wire.h>
#include <gossipd/gen_gossip_wire.h>
#include <lightningd/chaintopology.h>
#include <lightningd/jsonrpc.h>
@@ -204,6 +205,29 @@ static void gossip_dev_memleak_done(struct subd *gossipd,
report_leak_info(cmd, found_leak ? gossipd : NULL);
}
+static void connect_dev_memleak_done(struct subd *connectd,
+ const u8 *reply,
+ const int *fds UNUSED,
+ struct command *cmd)
+{
+ struct lightningd *ld = cmd->ld;
+ bool found_leak;
+
+ if (!fromwire_connect_dev_memleak_reply(reply, &found_leak)) {
+ command_fail(cmd, LIGHTNINGD, "Bad connect_dev_memleak");
+ return;
+ }
+
+ if (found_leak) {
+ report_leak_info(cmd, connectd);
+ return;
+ }
+
+ /* No leak? Ask gossipd. */
+ subd_req(ld->gossip, ld->gossip, take(towire_gossip_dev_memleak(NULL)),
+ -1, 0, gossip_dev_memleak_done, cmd);
+}
+
static void json_memleak(struct command *cmd,
const char *buffer UNNEEDED,
const jsmntok_t *params UNNEEDED)
@@ -219,8 +243,9 @@ static void json_memleak(struct command *cmd,
return;
}
- subd_req(ld->gossip, ld->gossip, take(towire_gossip_dev_memleak(NULL)),
- -1, 0, gossip_dev_memleak_done, cmd);
+ subd_req(ld->connectd, ld->connectd,
+ take(towire_connect_dev_memleak(NULL)),
+ -1, 0, connect_dev_memleak_done, cmd);
command_still_pending(cmd);
}