Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Aug 24, 2022

Diff to reproduce:

diff --git a/src/net.cpp b/src/net.cpp
index 865ce2ea97..ccf289d77b 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1150,6 +1150,7 @@ bool CConnman::InactivityCheck(const CNode& node) const
 
     if (last_recv.count() == 0 || last_send.count() == 0) {
         LogPrint(BCLog::NET, "socket no message in first %i seconds, %d %d peer=%d\n", count_seconds(m_peer_connect_timeout), last_recv.count() != 0, last_send.count() != 0, node.GetId());
+        UninterruptibleSleep(6s);
         return true;
     }

Example in CI:

 node0 2022-08-12T09:51:56.015288Z [net] [net.cpp:1152] [InactivityCheck] [net] socket no message in first 3 seconds, 0 0 peer=0 
 test  2022-08-12T09:51:57.658000Z TestFramework (ERROR): Assertion failed 
                                   Traceback (most recent call last):
                                     File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 133, in main
                                       self.run_test()
                                     File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_leak.py", line 155, in run_test
                                       assert not no_version_idle_peer.is_connected
                                   AssertionError

https://cirrus-ci.com/task/5346634421764096?logs=ci#L3683

@satsie
Copy link
Contributor

satsie commented Sep 3, 2022

ACK fa2aae5

Applied the 6s UninterruptibleSleep from the PR description, commented out the fix, and verified I could reproduce the failure. After putting back the changes from this PR, the test passes.

Copy link
Member

@luke-jr luke-jr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK fa2aae5

@fanquake fanquake merged commit 604015a into bitcoin:master Sep 4, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 4, 2022
fa2aae5 test: Fix intermittent issue in p2p_leak.py (MacroFake)

Pull request description:

  Diff to reproduce:

  ```diff
  diff --git a/src/net.cpp b/src/net.cpp
  index 865ce2e..ccf289d77b 100644
  --- a/src/net.cpp
  +++ b/src/net.cpp
  @@ -1150,6 +1150,7 @@ bool CConnman::InactivityCheck(const CNode& node) const

       if (last_recv.count() == 0 || last_send.count() == 0) {
           LogPrint(BCLog::NET, "socket no message in first %i seconds, %d %d peer=%d\n", count_seconds(m_peer_connect_timeout), last_recv.count() != 0, last_send.count() != 0, node.GetId());
  +        UninterruptibleSleep(6s);
           return true;
       }

  ```

  Example in CI:

  ```
   node0 2022-08-12T09:51:56.015288Z [net] [net.cpp:1152] [InactivityCheck] [net] socket no message in first 3 seconds, 0 0 peer=0
   test  2022-08-12T09:51:57.658000Z TestFramework (ERROR): Assertion failed
                                     Traceback (most recent call last):
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 133, in main
                                         self.run_test()
                                       File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_leak.py", line 155, in run_test
                                         assert not no_version_idle_peer.is_connected
                                     AssertionError
  ```

  https://cirrus-ci.com/task/5346634421764096?logs=ci#L3683

ACKs for top commit:
  satsie:
    ACK fa2aae5
  luke-jr:
    tACK fa2aae5

Tree-SHA512: e6ddf5b985f7da365b18b699ff8d0719b71b44e4e6bc5576d4099d1bad2c702495afd85f69f4edba89a883e13756a340946db2e7f4be41b1ac0e3c4f515ca4fd
@maflcko maflcko deleted the 2208-test-p2p-leak-int-🥇 branch September 5, 2022 07:19
@bitcoin bitcoin locked and limited conversation to collaborators Sep 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants