connectd: keep timeout timer around so we can disable it.

connectd will be keeping the conn open, so it needs to free this
"conn_timeout" timer.  Pass it through, so we can do that.

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

connectd/connectd.c

@@ -525,13 +525,14 @@ static struct io_plan *handshake_in_success(struct io_conn *conn,
 					    const struct pubkey *id_key,
 					    const struct wireaddr_internal *addr,
 					    struct crypto_state *cs,
+					    struct oneshot *timeout,
 					    struct daemon *daemon)
 {
 	struct node_id id;
 	node_id_from_pubkey(&id, id_key);
 	status_peer_debug(&id, "Connect IN");
 	return peer_exchange_initmsg(conn, daemon, daemon->our_features,
-				     cs, &id, addr, true);
+				     cs, &id, addr, timeout, true);
 }
 
 /*~ If the timer goes off, we simply free everything, which hangs up. */
@@ -591,11 +592,12 @@ static struct io_plan *conn_in(struct io_conn *conn,
 			       struct conn_in *conn_in_arg)
 {
 	struct daemon *daemon = conn_in_arg->daemon;
+	struct oneshot *timeout;
 
-	/* If they don't complete handshake in reasonable time, hang up */
-	notleak(new_reltimer(&daemon->timers, conn,
-			     time_from_sec(daemon->timeout_secs),
-			     conn_timeout, conn));
+	/* If they don't complete handshake in reasonable time, we hang up */
+	timeout = new_reltimer(&daemon->timers, conn,
+			       time_from_sec(daemon->timeout_secs),
+			       conn_timeout, conn);
 
 	/*~ The crypto handshake differs depending on whether you received or
 	 * initiated the socket connection, so there are two entry points.
@@ -603,7 +605,7 @@ static struct io_plan *conn_in(struct io_conn *conn,
 	 * code from thinking `conn` (which we don't keep a pointer to) is
 	 * leaked */
 	return responder_handshake(notleak(conn), &daemon->mykey,
-				   &conn_in_arg->addr,
+				   &conn_in_arg->addr, timeout,
 				   handshake_in_success, daemon);
 }
 
@@ -723,6 +725,7 @@ static struct io_plan *handshake_out_success(struct io_conn *conn,
 					     const struct pubkey *key,
 					     const struct wireaddr_internal *addr,
 					     struct crypto_state *cs,
+					     struct oneshot *timeout,
 					     struct connecting *connect)
 {
 	struct node_id id;
@@ -732,12 +735,13 @@ static struct io_plan *handshake_out_success(struct io_conn *conn,
 	status_peer_debug(&id, "Connect OUT");
 	return peer_exchange_initmsg(conn, connect->daemon,
 				     connect->daemon->our_features,
-				     cs, &id, addr, false);
+				     cs, &id, addr, timeout, false);
 }
 
 struct io_plan *connection_out(struct io_conn *conn, struct connecting *connect)
 {
 	struct pubkey outkey;
+	struct oneshot *timeout;
 
 	/* This shouldn't happen: lightningd should not give invalid ids! */
 	if (!pubkey_from_node_id(&outkey, &connect->id)) {
@@ -748,15 +752,15 @@ struct io_plan *connection_out(struct io_conn *conn, struct connecting *connect)
 	}
 
 	/* If they don't complete handshake in reasonable time, hang up */
-	notleak(new_reltimer(&connect->daemon->timers, conn,
-			     time_from_sec(connect->daemon->timeout_secs),
-			     conn_timeout, conn));
+	timeout = new_reltimer(&connect->daemon->timers, conn,
+			       time_from_sec(connect->daemon->timeout_secs),
+			       conn_timeout, conn);
 	status_peer_debug(&connect->id, "Connected out, starting crypto");
 
 	connect->connstate = "Cryptographic handshake";
 	return initiator_handshake(conn, &connect->daemon->mykey, &outkey,
 				   &connect->addrs[connect->addrnum],
-				   handshake_out_success, connect);
+				   timeout, handshake_out_success, connect);
 }
 
 /*~ When we've exhausted all addresses without success, we come here.

connectd/handshake.c

@@ -173,11 +173,15 @@ struct handshake {
 	/* Are we initiator or responder. */
 	enum bolt8_side side;
 
+	/* Timeout timer if we take too long. */
+	struct oneshot *timeout;
+
 	/* Function to call once handshake complete. */
 	struct io_plan *(*cb)(struct io_conn *conn,
 			      const struct pubkey *their_id,
 			      const struct wireaddr_internal *wireaddr,
 			      struct crypto_state *cs,
+			      struct oneshot *timeout,
 			      void *cbarg);
 	void *cbarg;
 };
@@ -348,10 +352,12 @@ static struct io_plan *handshake_succeeded(struct io_conn *conn,
 			      const struct pubkey *their_id,
 			      const struct wireaddr_internal *addr,
 			      struct crypto_state *cs,
+			      struct oneshot *timeout,
 			      void *cbarg);
 	void *cbarg;
 	struct pubkey their_id;
 	struct wireaddr_internal addr;
+	struct oneshot *timeout;
 
 	/* BOLT #8:
 	 *
@@ -377,9 +383,10 @@ static struct io_plan *handshake_succeeded(struct io_conn *conn,
 	cbarg = h->cbarg;
 	their_id = h->their_id;
 	addr = h->addr;
+	timeout = h->timeout;
 
 	tal_free(h);
-	return cb(conn, &their_id, &addr, &cs, cbarg);
+	return cb(conn, &their_id, &addr, &cs, timeout, cbarg);
 }
 
 static struct handshake *new_handshake(const tal_t *ctx,
@@ -956,10 +963,12 @@ static struct io_plan *act_one_responder(struct io_conn *conn,
 struct io_plan *responder_handshake_(struct io_conn *conn,
 				     const struct pubkey *my_id,
 				     const struct wireaddr_internal *addr,
+				     struct oneshot *timeout,
 				     struct io_plan *(*cb)(struct io_conn *,
 							   const struct pubkey *,
 							   const struct wireaddr_internal *,
 							   struct crypto_state *,
+							   struct oneshot *,
 							   void *cbarg),
 				     void *cbarg)
 {
@@ -970,6 +979,7 @@ struct io_plan *responder_handshake_(struct io_conn *conn,
 	h->addr = *addr;
 	h->cbarg = cbarg;
 	h->cb = cb;
+	h->timeout = timeout;
 
 	return act_one_responder(conn, h);
 }
@@ -978,10 +988,12 @@ struct io_plan *initiator_handshake_(struct io_conn *conn,
 				     const struct pubkey *my_id,
 				     const struct pubkey *their_id,
 				     const struct wireaddr_internal *addr,
+				     struct oneshot *timeout,
 				     struct io_plan *(*cb)(struct io_conn *,
 							   const struct pubkey *,
 							   const struct wireaddr_internal *,
 							   struct crypto_state *,
+							   struct oneshot *timeout,
 							   void *cbarg),
 				     void *cbarg)
 {
@@ -993,6 +1005,7 @@ struct io_plan *initiator_handshake_(struct io_conn *conn,
 	h->addr = *addr;
 	h->cbarg = cbarg;
 	h->cb = cb;
+	h->timeout = timeout;
 
 	return act_one_initiator(conn, h);
 }

connectd/handshake.h

@@ -6,15 +6,17 @@ struct crypto_state;
 struct io_conn;
 struct wireaddr_internal;
 struct pubkey;
+struct oneshot;
 
-#define initiator_handshake(conn, my_id, their_id, addr, cb, cbarg)	\
-	initiator_handshake_((conn), (my_id), (their_id), (addr),	\
+#define initiator_handshake(conn, my_id, their_id, addr, timeout, cb, cbarg) \
+	initiator_handshake_((conn), (my_id), (their_id), (addr), (timeout), \
 			     typesafe_cb_preargs(struct io_plan *, void *, \
 						 (cb), (cbarg),		\
 						 struct io_conn *,	\
 						 const struct pubkey *,	\
-						 const struct wireaddr_internal *,	\
-						 struct crypto_state *), \
+						 const struct wireaddr_internal *, \
+						 struct crypto_state *, \
+						 struct oneshot *),	\
 			     (cbarg))
 
 
@@ -22,31 +24,36 @@ struct io_plan *initiator_handshake_(struct io_conn *conn,
 				     const struct pubkey *my_id,
 				     const struct pubkey *their_id,
 				     const struct wireaddr_internal *addr,
+				     struct oneshot *timeout,
 				     struct io_plan *(*cb)(struct io_conn *,
 							   const struct pubkey *,
 							   const struct wireaddr_internal *,
 							   struct crypto_state *,
+							   struct oneshot *timeout,
 							   void *cbarg),
 				     void *cbarg);
 
 
-#define responder_handshake(conn, my_id, addr, cb, cbarg)		\
-	responder_handshake_((conn), (my_id), (addr),			\
+#define responder_handshake(conn, my_id, addr, timeout, cb, cbarg)	\
+	responder_handshake_((conn), (my_id), (addr), (timeout),	\
 			     typesafe_cb_preargs(struct io_plan *, void *, \
 						 (cb), (cbarg),		\
 						 struct io_conn *,	\
 						 const struct pubkey *,	\
-						 const struct wireaddr_internal *,	\
-						 struct crypto_state *), \
+						 const struct wireaddr_internal *, \
+						 struct crypto_state *, \
+						 struct oneshot *),	\
 			     (cbarg))
 
 struct io_plan *responder_handshake_(struct io_conn *conn,
 				     const struct pubkey *my_id,
 				     const struct wireaddr_internal *addr,
+				     struct oneshot *timeout,
 				     struct io_plan *(*cb)(struct io_conn *,
 							   const struct pubkey *,
 							   const struct wireaddr_internal *,
 							   struct crypto_state *,
+							   struct oneshot *,
 							   void *cbarg),
 				     void *cbarg);
 #endif /* LIGHTNING_CONNECTD_HANDSHAKE_H */

connectd/peer_exchange_initmsg.c

@@ -2,6 +2,7 @@
 #include <bitcoin/chainparams.h>
 #include <ccan/io/io.h>
 #include <common/dev_disconnect.h>
+#include <common/memleak.h>
 #include <common/status.h>
 #include <common/wire_error.h>
 #include <connectd/connectd.h>
@@ -151,6 +152,7 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn,
 				      const struct crypto_state *cs,
 				      const struct node_id *id,
 				      const struct wireaddr_internal *addr,
+				      struct oneshot *timeout,
 				      bool incoming)
 {
 	/* If conn is closed, forget peer */
@@ -164,6 +166,9 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn,
 	peer->cs = *cs;
 	peer->incoming = incoming;
 
+	/* Attach timer to early peer, so it gets freed with it. */
+	notleak(tal_steal(peer, timeout));
+
 	/* BOLT #1:
 	 *
 	 * The sending node:

connectd/peer_exchange_initmsg.h

@@ -8,6 +8,7 @@ struct daemon;
 struct io_conn;
 struct node_id;
 struct wireaddr_internal;
+struct oneshot;
 
 /* If successful, calls peer_connected() */
 struct io_plan *peer_exchange_initmsg(struct io_conn *conn,
@@ -16,6 +17,7 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn,
 				      const struct crypto_state *cs,
 				      const struct node_id *id,
 				      const struct wireaddr_internal *addr,
+				      struct oneshot *timeout,
 				      bool incoming);
 
 #endif /* LIGHTNING_CONNECTD_PEER_EXCHANGE_INITMSG_H */

connectd/test/run-initiator-success.c

@@ -278,6 +278,7 @@ static struct io_plan *success(struct io_conn *conn UNUSED,
 			       const struct pubkey *them,
 			       const struct wireaddr_internal *addr UNUSED,
 			       struct crypto_state *cs,
+			       struct oneshot *timeout UNUSED,
 			       void *unused UNUSED)
 {
 	assert(pubkey_eq(them, &rs_pub));
@@ -320,7 +321,7 @@ int main(int argc, char *argv[])
 
 	dummy.itype = ADDR_INTERNAL_WIREADDR;
 	dummy.u.wireaddr.addrlen = 0;
-	initiator_handshake((void *)tmpctx, &ls_pub, &rs_pub, &dummy, success, NULL);
+	initiator_handshake((void *)tmpctx, &ls_pub, &rs_pub, &dummy, NULL, success, NULL);
 	/* Should not exit! */
 	abort();
 }

connectd/test/run-responder-success.c

@@ -277,6 +277,7 @@ static struct io_plan *success(struct io_conn *conn UNUSED,
 			       const struct pubkey *them UNUSED,
 			       const struct wireaddr_internal *addr UNUSED,
 			       struct crypto_state *cs,
+			       struct oneshot *timeout UNUSED,
 			       void *unused UNUSED)
 {
 	assert(secret_eq_str(&cs->sk, expect_sk));
@@ -314,7 +315,7 @@ int main(int argc, char *argv[])
 
 	dummy.itype = ADDR_INTERNAL_WIREADDR;
 	dummy.u.wireaddr.addrlen = 0;
-	responder_handshake((void *)tmpctx, &ls_pub, &dummy, success, NULL);
+	responder_handshake((void *)tmpctx, &ls_pub, &dummy, NULL, success, NULL);
 	/* Should not exit! */
 	abort();
 }

devtools/gossipwith.c

@@ -144,6 +144,7 @@ static struct io_plan *handshake_success(struct io_conn *conn,
 					 const struct pubkey *them,
 					 const struct wireaddr_internal *addr,
 					 struct crypto_state *orig_cs,
+					 struct oneshot *timer,
 					 char **args)
 {
 	u8 *msg;
@@ -351,7 +352,8 @@ int main(int argc, char *argv[])
 	if (connect(conn->fd, ai->ai_addr, ai->ai_addrlen) != 0)
 		err(1, "Connecting to %s", at+1);
 
-	initiator_handshake(conn, &us, &them, &addr, handshake_success, argv+2);
+	initiator_handshake(conn, &us, &them, &addr, NULL,
+			    handshake_success, argv+2);
 	exit(0);
 }