test_bitcoin and caddrdb_read failures #8069

issue paveljanik openend this issue on May 18, 2016
  1. paveljanik commented at 7:35 am on May 18, 2016: contributor

    Current master:

    0Running 194 test cases...
    1test/net_tests.cpp:94: error in "caddrdb_read": check addrman1.size() == 3 failed
    2test/net_tests.cpp:105: error in "caddrdb_read": check addrman2.size() == 3 failed
    3
    4*** 2 failures detected in test suite "Bitcoin Test Suite"
    

    @EthanHeilman ?

    I extended a debugging:

     0diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
     1index 6debf6a..64afbad 100644
     2--- a/src/test/net_tests.cpp
     3+++ b/src/test/net_tests.cpp
     4@@ -57,6 +57,8 @@ CDataStream AddrmanToStream(CAddrManSerializationMock& addrman)
     5     ssPeersIn << addrman;
     6     std::string str = ssPeersIn.str();
     7     vector<unsigned char> vchData(str.begin(), str.end());
     8+    cerr << "DBG: vchData.size() = " << vchData.size();
     9+    cerr << ", vchData = " << HexStr(vchData) << "\n";
    10     return CDataStream(vchData, SER_DISK, CLIENT_VERSION);
    11 }
    12
    13@@ -89,6 +91,10 @@ BOOST_AUTO_TEST_CASE(caddrdb_read)
    14         exceptionThrown = true;
    15     }
    16
    17+    if (addrman1.size() != 3)
    18+        cerr << "addrman1.size() == " << addrman1.size() << "\n";
    19+    if (exceptionThrown)
    20+        cerr << "exceptionThrown!\n";
    21     BOOST_CHECK(addrman1.size() == 3);
    22     BOOST_CHECK(exceptionThrown == false);
    23
    24@@ -99,6 +105,8 @@ BOOST_AUTO_TEST_CASE(caddrdb_read)
    25     CAddrDB adb;
    26     BOOST_CHECK(addrman2.size() == 0);
    27     adb.Read(addrman2, ssPeers2);
    28+    if (addrman2.size() != 3)
    29+        cerr << "addrman2.size() == " << addrman2.size();
    30     BOOST_CHECK(addrman2.size() == 3);
    31 }
    

    And to reproduce the problem:

    0for i in {1..100}; do ./test_bitcoin --run_test=net_tests || break ; done
    

    It looks like the serialization of CAddrMan is fragile…

  2. paveljanik commented at 7:45 am on May 18, 2016: contributor
    The nNew inside serialized format is 3 vs 2 in such simple case. Why?
  3. paveljanik commented at 8:01 am on May 18, 2016: contributor
    Hmm, in the fail case, after the third entry added to the addrmanUncorrupted (https://github.com/bitcoin/bitcoin/blob/master/src/test/net_tests.cpp#L76), the size of addrmanUncorrupted is still two! So it is not serialization related!
  4. paveljanik commented at 8:03 am on May 18, 2016: contributor
    This is new test added in the last merge commit #7696.
  5. paveljanik commented at 8:22 am on May 18, 2016: contributor

    I can reproduce the problem in plain master with addrman_simple:

    0for i in {1..100}; do ./test_bitcoin --run_test=addrman_tests || break; done
    1...
    2Running 11 test cases...
    3test/addrman_tests.cpp:87: error in "addrman_simple": check addrman.size() == 2 failed
    4
    5*** 1 failure detected in test suite "Bitcoin Test Suite"
    
  6. EthanHeilman commented at 2:40 pm on May 18, 2016: contributor
    @paveljanik Looking at this now.
  7. MarcoFalke added the label Tests on May 18, 2016
  8. EthanHeilman commented at 4:15 pm on May 18, 2016: contributor

    In regards to the net_tests failing: Addrman will evict an existing addr if it is in the same position as an existing addr. This position is determined via a hash function and a randomly picked seed (nKey). This seed is serialized and deserialized so if the Addrman which is serialized to the disk has a randomly chosen seed this will cause non-determinism down the road when it is deserialized and tested.

    For testing purposes I created a deterministic subclass of Addrman to prevent exactly these problems but it was not used in this test. I have created a pull request which resolves this issue: #8070

    In regards to the addrman_tests failing: Unlike the net_tests failure I’m unable to replicate addrman_tests failures and addrman_tests does not have the same issue as net_tests. I ran your script several times without a failure.

    0for i in {1..100}; do ./test_bitcoin --run_test=addrman_tests || break; done
    1Running 11 test cases...
    2...
    3*** No errors detected
    

    Now testing for i in {1..10000}; will post results when it finished. What system are you running?

  9. EthanHeilman commented at 7:35 pm on May 18, 2016: contributor

    Ran addrman_tests 10000 times no failures on my side:

    0for i in {1..10000}; do echo $i;./test_bitcoin --run_test=addrman_tests || break; done
    1...
    2*** No errors detected
    39999
    4Running 11 test cases...
    5*** No errors detected
    610000
    7Running 11 test cases...
    8*** No errors detected
    
  10. paveljanik commented at 7:45 pm on May 18, 2016: contributor

    @EthanHeilman The same for me now. Strange. Will investigate.

    Maybe it was a false alarm when I was debugging the net_tests problem - I could have removed MakeDeterministic() calls there…

    I can’t reproduce addrman_tests issue on plain master.

    But anyway, the original problem is fixed by #8070. Thanks!

  11. laanwj referenced this in commit 7771aa57bd on May 19, 2016
  12. paveljanik commented at 7:58 am on May 19, 2016: contributor
    Fixed by #8070, closing.
  13. paveljanik closed this on May 19, 2016

  14. sickpig referenced this in commit 16969f5d33 on May 2, 2017
  15. OlegGirko referenced this in commit 863755e828 on Jun 7, 2017
  16. dexX7 referenced this in commit a400d26561 on Jun 8, 2017
  17. dexX7 referenced this in commit 2476c64723 on Jun 8, 2017
  18. dexX7 referenced this in commit 3edbe7cc79 on Jun 8, 2017
  19. OlegGirko referenced this in commit 495e3c1d0c on Jun 13, 2017
  20. OlegGirko referenced this in commit 0ed000cdd9 on Jun 16, 2017
  21. OlegGirko referenced this in commit 6e9c09d952 on Jun 23, 2017
  22. OlegGirko referenced this in commit c45adc812a on Jun 26, 2017
  23. OlegGirko referenced this in commit c1a2dcdabb on Jun 29, 2017
  24. OlegGirko referenced this in commit a64b97a153 on Jun 29, 2017
  25. OlegGirko referenced this in commit 9b0a8a0e1e on Jun 29, 2017
  26. OlegGirko referenced this in commit 664e1aefca on Jun 30, 2017
  27. OlegGirko referenced this in commit b31ab5e7c2 on Jul 2, 2017
  28. OlegGirko referenced this in commit 7667db59f2 on Jul 3, 2017
  29. OlegGirko referenced this in commit 94969beccc on Jul 4, 2017
  30. OlegGirko referenced this in commit cd80181e0c on Jul 4, 2017
  31. OlegGirko referenced this in commit b480ab4c04 on Jul 5, 2017
  32. OlegGirko referenced this in commit c447a5a6ef on Jul 5, 2017
  33. OlegGirko referenced this in commit f73b9fa288 on Jul 5, 2017
  34. OlegGirko referenced this in commit bca19bc8df on Jul 9, 2017
  35. UdjinM6 referenced this in commit b47984f303 on Jul 10, 2017
  36. codablock referenced this in commit 03e8f777c9 on Sep 16, 2017
  37. codablock referenced this in commit 4bafc7f5ab on Sep 19, 2017
  38. lateminer referenced this in commit 90342d8b9c on Nov 11, 2018
  39. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-04 22:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me