This makes several improvements to the way we close the p2p pool.
Make the peer close function synchronous by not awaiting the disconnection reason packet being sent. Previously we would wait and hold up the closing flow, which could introduce delays on stale or dead sockets. It also meant there was different behavior for closing peers with or without a disconnection reason. Instead we continue with all the steps for closing a peer and destroy the socket whenever the socket write attempt is complete.
Immediately destroy the socket if it is in connecting state, don't even attempt to send a disconnection reason packet since it
would not work anyway.
Don't increment the connection failure attempt counter for peers when/after we close the pool and thus are canceling any ongoing connection attempts. This goes against the spirit of the connection failure counter and also means that he pool may be making db writes after it is closed (and potentially after the database has closed).
Then in the Pool test suite, this flips the order of the db close and the pool close so that the pool is second, which makes sense
since it depends on an open, functioning database. It also moves the logic in the before block up to the describe block, the before block wasn't properly completing before the test cases began running. See https://mochajs.org/#asynchronous-code.
This makes several improvements to the way we close the p2p pool.
Make the peer
closefunction synchronous by not awaiting the disconnection reason packet being sent. Previously we would wait and hold up the closing flow, which could introduce delays on stale or dead sockets. It also meant there was different behavior for closing peers with or without a disconnection reason. Instead we continue with all the steps for closing a peer and destroy the socket whenever the socket write attempt is complete.Immediately destroy the socket if it is in
connectingstate, don't even attempt to send a disconnection reason packet since it would not work anyway.Don't increment the connection failure attempt counter for peers when/after we close the pool and thus are canceling any ongoing connection attempts. This goes against the spirit of the connection failure counter and also means that he pool may be making db writes after it is closed (and potentially after the database has closed).
Then in the Pool test suite, this flips the order of the db close and the pool close so that the pool is second, which makes sense since it depends on an open, functioning database. It also moves the logic in the
beforeblock up to thedescribeblock, thebeforeblock wasn't properly completing before the test cases began running. See https://mochajs.org/#asynchronous-code.This fixes the p2p pool test flakiness issue described here: https://github.com/ExchangeUnion/xud/issues/1771#issuecomment-732289609.