Skip to content

Commit

Permalink
Merge bitcoin#29898: test: Fix intermittent issue in p2p_handshake.py
Browse files Browse the repository at this point in the history
6b02c11 test: Fix intermittent issue in p2p_handshake.py (stratospher)

Pull request description:

  When establishing outbound connections [`TestNode` --------> `P2PConnection`], `P2PConnection` listens for a single connection from `TestNode` on a [port which is fixed based on `p2p_idx`](https://github.com/bitcoin/bitcoin/blob/312f54278fd972ba3557c6a5b805fd244a063959/test/functional/test_framework/p2p.py#L746).

  If we reuse the same port when disconnecting and establishing connections again, we might hit this scenario where:
  - disconnection is done on python side for `P2PConnection`
  - disconnection not complete on c++ side for `TestNode`
  - we're trying to establish a new connection on same port again

  Prevent this scenario from happening by ensuring disconnection on c++ side for TestNode as well.

  One way to reproduce this on master would be adding a sleep statement before disconnection happens on c++ side.

  ```diff
  diff --git a/src/net.cpp b/src/net.cpp
  index e388f05b03..62507d1f39 100644
  --- a/src/net.cpp
  +++ b/src/net.cpp
  @@ -2112,6 +2112,7 @@ void CConnman::SocketHandlerConnected(const std::vector<CNode*>& nodes,
                   if (!pnode->fDisconnect) {
                       LogPrint(BCLog::NET, "socket closed for peer=%d\n", pnode->GetId());
                   }
  +                std::this_thread::sleep_for(std::chrono::milliseconds(1000));
                   pnode->CloseSocketDisconnect();
               }
               else if (nBytes < 0)
  ```

ACKs for top commit:
  maflcko:
    lgtm ACK 6b02c11
  mzumsande:
    Tested ACK 6b02c11
  BrandonOdiwuor:
    Tested ACK 6b02c11
  theStack:
    Tested ACK 6b02c11
  glozow:
    ACK 6b02c11

Tree-SHA512: 69509edb61ba45739fd585b6cc8a254f412975c124a5b5a52688288ecaaffd264dd76019b8290cc34c26c3ac2dfe477965ee5a11d7aabdd8e4d2a75229a4a068
  • Loading branch information
glozow committed Apr 22, 2024
2 parents 04c90f1 + 6b02c11 commit b3106be
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions test/functional/p2p_handshake.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def add_outbound_connection(self, node, connection_type, services, wait_for_disc
peer.sync_with_ping()
peer.peer_disconnect()
peer.wait_for_disconnect()
self.wait_until(lambda: len(node.getpeerinfo()) == 0)

def test_desirable_service_flags(self, node, service_flag_tests, desirable_service_flags, expect_disconnect):
"""Check that connecting to a peer either fails or succeeds depending on its offered
Expand Down

0 comments on commit b3106be

Please sign in to comment.