Don't hammer disconnecting peers

[buck54321]

A peer disconnect can trigger immediate reconnects to the same peer regardless of the nature of the disconnect.

Starting at (*Peer).Disconnect, closing the quit channel causes WaitForDisconnect to return in peerDoneHandler, which sends a message to handleDonePeerMsg. For non-persistent peers, this can cause two simultanous calls to GetNewAddress. Once more directly via NewConnReq, and once via (*ConnManager).Remove sending a handleDisconnected to the (*ConnManager).connHandler and (*ConnManager).handleFailedConn, which calls NewConnReq again. So a disconnecting peer can trigger two new connection requests immediately, and there's nothing preventing the same address from being used.

The newAddressFunc used by *server in btcd may also benefit from this change. I think it might be the reason for the // TODO: duplicate oneshots? comment.

[Chinwendu20]

If I understand correctly, you are saying that disconnecting a peer can lead to immediate reconnection.
You went further to state that this connection request happens twice for non persistent peers as disconnecting them would lead to a call to GetNewAddressFunc twice?

1. As a result of the call to the Remove method of the connmanager
2. Then here as well when a NewConnReq method is called directly

Right?
For the first case, this specifically occurs during a call to the handleFailedConn here

Are my following correctly?

[buck54321]

Yes. That's correct.

[Chinwendu20]

Thanks, I do not think the first case happens though because of this conditional:
https://github.com/btcsuite/btcd/blob/6b197d38d745048c5fe2a895010c9c0a4d9ab2a6/connmgr/connmanager.go#L319-L322

The Remove method sets the retry bool as false:
https://github.com/btcsuite/btcd/blob/6b197d38d745048c5fe2a895010c9c0a4d9ab2a6/connmgr/connmanager.go#L489

[buck54321]

Oooh. OK. Hmm. I'm seeing little clusters of calls to the same address. Up to 5 at a time unmetered on a loosely connected testnet. I've traced the clusters to mostly handleFailedConn and handleDonePeerMsg buy maybe I'm misunderstanding the nature. Lemme look a little closer.

[buck54321]

I can't reliably reproduce. Will reopen if I figure something out.