From d4c8210a9e7004087b4874acbd8df90013e94ee7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 10 Jan 2018 16:00:54 +1030 Subject: [PATCH] gossipd: don't hang if we try to connect to already-connected peer. Closes: #287 Signed-off-by: Rusty Russell --- gossipd/gossip.c | 20 ++++++++++++++------ gossipd/gossip_wire.csv | 7 ++++++- lightningd/gossip_control.c | 3 +++ lightningd/peer_control.c | 13 +++++++++++++ lightningd/peer_control.h | 3 +++ 5 files changed, 39 insertions(+), 7 deletions(-) diff --git a/gossipd/gossip.c b/gossipd/gossip.c index 255af2620..d4f942b38 100644 --- a/gossipd/gossip.c +++ b/gossipd/gossip.c @@ -167,7 +167,7 @@ static struct io_plan *peer_start_gossip(struct io_conn *conn, struct peer *peer); static bool send_peer_with_fds(struct peer *peer, const u8 *msg); static void wake_pkt_out(struct peer *peer); -static void try_reach_peer(struct daemon *daemon, const struct pubkey *id); +static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id); static void destroy_peer(struct peer *peer) { @@ -1474,16 +1474,17 @@ static void try_connect(struct reaching *reach) io_new_conn(reach, fd, conn_init, reach); } -static void try_reach_peer(struct daemon *daemon, const struct pubkey *id) +/* Returns true if we're already connected. */ +static bool try_reach_peer(struct daemon *daemon, const struct pubkey *id) { struct reaching *reach; struct peer *peer; if (find_reaching(daemon, id)) { /* FIXME: Perhaps kick timer in this case? */ - status_trace("try_reach_peer: already reaching %s", + status_trace("try_reach_peer: already trying to reach %s", type_to_string(trc, struct pubkey, id)); - return; + return false; } /* Master might find out before we do that a peer is dead; if we @@ -1493,7 +1494,7 @@ static void try_reach_peer(struct daemon *daemon, const struct pubkey *id) status_trace("reach_peer: have %s, will retry if it dies", type_to_string(trc, struct pubkey, id)); peer->reach_again = true; - return; + return true; } reach = tal(daemon, struct reaching); @@ -1504,6 +1505,7 @@ static void try_reach_peer(struct daemon *daemon, const struct pubkey *id) tal_add_destructor(reach, destroy_reaching); try_connect(reach); + return false; } /* This catches all kinds of failures, like network errors. */ @@ -1515,7 +1517,12 @@ static struct io_plan *reach_peer(struct io_conn *conn, if (!fromwire_gossipctl_reach_peer(msg, NULL, &id)) master_badmsg(WIRE_GOSSIPCTL_REACH_PEER, msg); - try_reach_peer(daemon, &id); + /* Master can't check this itself, because that's racy. */ + if (try_reach_peer(daemon, &id)) { + daemon_conn_send(&daemon->master, + take(towire_gossip_peer_already_connected(conn, + &id))); + } return daemon_conn_read_next(conn, &daemon->master); } @@ -1632,6 +1639,7 @@ static struct io_plan *recv_req(struct io_conn *conn, struct daemon_conn *master case WIRE_GOSSIP_PING_REPLY: case WIRE_GOSSIP_RESOLVE_CHANNEL_REPLY: case WIRE_GOSSIP_PEER_CONNECTED: + case WIRE_GOSSIP_PEER_ALREADY_CONNECTED: case WIRE_GOSSIP_PEER_NONGOSSIP: case WIRE_GOSSIP_GET_UPDATE: case WIRE_GOSSIP_GET_UPDATE_REPLY: diff --git a/gossipd/gossip_wire.csv b/gossipd/gossip_wire.csv index f9782ccd5..c2804a765 100644 --- a/gossipd/gossip_wire.csv +++ b/gossipd/gossip_wire.csv @@ -22,7 +22,8 @@ gossipctl_peer_addrhint,3014 gossipctl_peer_addrhint,,id,struct pubkey gossipctl_peer_addrhint,,addr,struct wireaddr -# Master -> gossipd: connect to a peer. We may get a peer_connected. +# Master -> gossipd: connect to a peer. We may get a peer_connected or +# peer_already_connected gossipctl_reach_peer,3001 gossipctl_reach_peer,,id,struct pubkey @@ -37,6 +38,10 @@ gossip_peer_connected,,gfeatures,gflen*u8 gossip_peer_connected,,lflen,u16 gossip_peer_connected,,lfeatures,lflen*u8 +# Gossipd -> master: you asked to reach a peer, we already had. +gossip_peer_already_connected,3015 +gossip_peer_already_connected,,id,struct pubkey + # Gossipd -> master: peer sent non-gossip packet. Two fds: peer and gossip gossip_peer_nongossip,3003 gossip_peer_nongossip,,id,struct pubkey diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 1f5b08dc9..2dbf23f27 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -120,6 +120,9 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) return 2; peer_connected(gossip->ld, msg, fds[0], fds[1]); break; + case WIRE_GOSSIP_PEER_ALREADY_CONNECTED: + peer_already_connected(gossip->ld, msg); + break; case WIRE_GOSSIP_PEER_NONGOSSIP: if (tal_count(fds) != 2) return 2; diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 63ab8b254..dc5db5937 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -667,6 +667,19 @@ send_error: subd_send_fd(ld->gossip, gossip_fd); } +/* Gossipd tells us peer was already connected. */ +void peer_already_connected(struct lightningd *ld, const u8 *msg) +{ + struct pubkey id; + + if (!fromwire_gossip_peer_already_connected(msg, NULL, &id)) + fatal("Gossip gave bad GOSSIP_PEER_ALREADY_CONNECTED message %s", + tal_hex(msg, msg)); + + /* If we were waiting for connection, we succeeded. */ + connect_succeeded(ld, &id); +} + void peer_sent_nongossip(struct lightningd *ld, const struct pubkey *id, const struct wireaddr *addr, diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index 548af4932..7b58cfc5c 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -173,6 +173,9 @@ void peer_last_tx(struct peer *peer, struct bitcoin_tx *tx, void peer_connected(struct lightningd *ld, const u8 *msg, int peer_fd, int gossip_fd); +/* This simply means we asked to reach a peer, but we already have it */ +void peer_already_connected(struct lightningd *ld, const u8 *msg); + void peer_sent_nongossip(struct lightningd *ld, const struct pubkey *id, const struct wireaddr *addr,