travis: Enable functional tests in the ThreadSanitizer (TSan) build job #14829

pull practicalswift wants to merge 3 commits into bitcoin:master from practicalswift:tsan changing 4 files +13 −11
  1. practicalswift commented at 8:32 PM on November 28, 2018: contributor

    Enable functional tests in the ThreadSanitizer (TSan) build job.

    This is a follow-up to @MarcoFalke's #14764 which added TSan but for unit tests only.

  2. DrahtBot commented at 11:21 PM on November 28, 2018: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #14841 (consensus: Move CheckBlock() call to critical section by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. fanquake added the label Tests on Nov 29, 2018
  4. fanquake requested review from MarcoFalke on Nov 29, 2018
  5. in .travis.yml:102 in 1fac48773a outdated
      98 | @@ -99,9 +99,9 @@ jobs:
      99 |          DOCKER_NAME_TAG=ubuntu:16.04
     100 |          PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libssl-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libprotobuf-dev protobuf-compiler libqrencode-dev"
     101 |          NO_DEPENDS=1
     102 | -        RUN_FUNCTIONAL_TESTS=false # Disabled for now. TODO identify suppressions or exclude specific tests
     103 | +        FUNCTIONAL_TESTS_CONFIG="--exclude feature_block.py,p2p_invalid_messages.py"
    


    MarcoFalke commented at 8:19 PM on November 29, 2018:

    Why are they excluded?


    practicalswift commented at 10:31 AM on December 3, 2018:

    feature_block.py and p2p_invalid_message.py fail for some unknown reason. Unfortunately the error log is not very informative so I haven't figured out why or what suppression (if any) would solve it.


    MarcoFalke commented at 6:06 PM on December 3, 2018:

    They are timing out because the tsan slows down bitcoind so much. You'd probably have to up all the various timeouts (rpcwait and the timeout for polling loops)


    MarcoFalke commented at 6:07 PM on December 3, 2018:

    At least that is what I assume based on my tsan runs a few days ago


    practicalswift commented at 7:07 PM on December 3, 2018:

    @MarcoFalke Ah, I see. What do you think about doing that in a follow-up PR once this is merged? It would be nice to keep this initial PR minimal to get basic functional testing under Travis. Debugging Travis is quite time consuming so I'd rather split the task in two if possible.

  6. in .travis.yml:104 in 1fac48773a outdated
      98 | @@ -99,9 +99,9 @@ jobs:
      99 |          DOCKER_NAME_TAG=ubuntu:16.04
     100 |          PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libssl-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libprotobuf-dev protobuf-compiler libqrencode-dev"
     101 |          NO_DEPENDS=1
     102 | -        RUN_FUNCTIONAL_TESTS=false # Disabled for now. TODO identify suppressions or exclude specific tests
     103 | +        FUNCTIONAL_TESTS_CONFIG="--exclude feature_block.py,p2p_invalid_messages.py"
     104 |          GOAL="install"
     105 | -        BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER --with-sanitizers=thread --disable-hardening --disable-asm CC=clang CXX=clang++"
     106 | +        BITCOIN_CONFIG="--enable-zmq --disable-wallet --with-gui=qt5 --with-sanitizers=thread --disable-hardening --disable-asm CC=clang CXX=clang++"
    


    MarcoFalke commented at 8:20 PM on November 29, 2018:

    Why is the wallet and DEBUG_LOCKORDER disabled?


    practicalswift commented at 10:29 AM on December 3, 2018:

    For some strange reasons all functional wallets tests fail under Travis (wallet_basic, wallet_dump, etc.). The failure reason is not clear from the logging so I haven't been able to solve this yet.


    practicalswift commented at 1:40 PM on December 3, 2018:

    I've now re-added CPPFLAGS=-DDEBUG_LOCKORDER since it doesn't seem to cause any problems.


    MarcoFalke commented at 8:25 PM on December 3, 2018:

    Looks like they time out with

    No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

    https://travis-ci.org/MarcoFalke/bitcoin/jobs/462970867

    which shouldn't happen, since we should be printing dots all the time. Not sure what is going on :(

  7. in test/sanitizer_suppressions/tsan:26 in 1fac48773a outdated
      29 | +deadlock:WalletBatch::WritePool
      30 | +deadlock:WalletBatch::WritePurpose
      31 | +deadlock:WalletBatch::WriteTx
      32 | +deadlock:WalletBatch::WriteWatchOnly
      33 | +race:BerkeleyDatabase::Flush
      34 | +race:BerkeleyEnvironment::Flush
    


    MarcoFalke commented at 8:22 PM on November 29, 2018:

    This seems like a lot, what about something like this instead:

    diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan
    index 209c46f..4edf68c 100644
    --- a/test/sanitizer_suppressions/tsan
    +++ b/test/sanitizer_suppressions/tsan
    @@ -14,9 +14,10 @@ deadlock:TestPotentialDeadLockDetected
     race:src/qt/test/*
     deadlock:src/qt/test/*
     
    -# WIP: Unidentified suppressions to run the functional tests
    -#race:zmqpublishnotifier.cpp
    -#
    +# External libraries
    +deadlock:libdb
    +race:libzmq
    +
    

    practicalswift commented at 8:10 AM on December 3, 2018:

    Fixed!

  8. DrahtBot added the label Needs rebase on Dec 1, 2018
  9. practicalswift force-pushed on Dec 2, 2018
  10. practicalswift force-pushed on Dec 2, 2018
  11. practicalswift force-pushed on Dec 3, 2018
  12. practicalswift force-pushed on Dec 3, 2018
  13. practicalswift force-pushed on Dec 3, 2018
  14. practicalswift commented at 1:41 PM on December 3, 2018: contributor

    @MarcoFalke Please review the latest version.

    I've now re-introduced CPPFLAGS=-DDEBUG_LOCKORDER.

    However, I've been unable to make the Travis build pass without FUNCTIONAL_TESTS_CONFIG="--exclude feature_block.py,p2p_invalid_messages.py" and --disable-wallet.

  15. practicalswift force-pushed on Dec 3, 2018
  16. in .travis/test_06_script.sh:48 in 167ad3c884 outdated
      40 | @@ -41,12 +41,19 @@ DOCKER_EXEC ./configure --cache-file=../config.cache $BITCOIN_CONFIG_ALL $BITCOI
      41 |  END_FOLD
      42 |  
      43 |  BEGIN_FOLD build
      44 | -DOCKER_EXEC make $MAKEJOBS $GOAL || ( echo "Build failure. Verbose build follows." && DOCKER_EXEC make $GOAL V=1 ; false )
      45 | +DOCKER_EXEC make $MAKEJOBS $GOAL || (
      46 | +  echo "Build failure. Verbose build follows." && DOCKER_EXEC make $GOAL V=1
      47 | +  DOCKER_EXEC "cat ${TRAVIS_BUILD_DIR}/sanitizer-output/* 2> /dev/null"
      48 | +  false
      49 | +)
    


    ken2812221 commented at 2:01 PM on December 3, 2018:

    Could these commands be run in after_failure step? Then you don't have to copy-paste these code in three different places.


    practicalswift commented at 7:14 PM on December 3, 2018:

    @ken2812221 I haven't tried after_failure before, but let me try to summarise: the suggestion is to add script say .travis/test_XX_after_failure.sh containing ...

    #!/bin/bash
    
    DOCKER_EXEC "cat ${TRAVIS_BUILD_DIR}/sanitizer-output/* 2> /dev/null"
    

    .. that would be referenced using ...

    after_failure:
      - set -o errexit; source .travis/test_XX_after_failure.sh
    

    … in .travis.yml?

    Is that a correct summary of the suggestion? :-)


    practicalswift commented at 3:13 PM on December 14, 2018:

    @ken2812221 Friendly ping :-)


    MarcoFalke commented at 10:51 PM on December 14, 2018:

    Could just test it?


    MarcoFalke commented at 10:55 PM on December 14, 2018:

    Ah, I guess it wouldn't work because we exit "hard" as opposed to just return a non-zero exit code.


    ken2812221 commented at 2:42 AM on December 15, 2018:

    The trap way seems to work. 8d78bb9536e44e6b79532f539b1dd0066ce7ee6c

    https://travis-ci.org/ken2812221/bitcoin/jobs/468068901

  17. DrahtBot removed the label Needs rebase on Dec 3, 2018
  18. practicalswift commented at 3:25 PM on December 15, 2018: contributor

    @MarcoFalke @ken2812221 Updated. Please re-review :-)

  19. build: Enable functional tests in the ThreadSanitizer (TSan) build job 069752b726
  20. travis: Use trap and set -e errtrace 5e5138a721
  21. practicalswift force-pushed on Dec 17, 2018
  22. practicalswift force-pushed on Dec 17, 2018
  23. MarcoFalke commented at 10:04 PM on December 17, 2018: member

    utACK 5e5138a721738f47053d915e4c65f925838ad5b4

  24. Add suppression for InterruptRPC (fRPCRunning) data race eaf4070e3a
  25. practicalswift force-pushed on Dec 18, 2018
  26. practicalswift commented at 9:19 AM on December 18, 2018: contributor

    @MarcoFalke Added suppression (race:InterruptRPC, fix in #14993). Please re-review :-)

  27. MarcoFalke merged this on Dec 18, 2018
  28. MarcoFalke closed this on Dec 18, 2018

  29. MarcoFalke referenced this in commit f055389cb9 on Dec 18, 2018
  30. MarcoFalke referenced this in commit 7027c67cac on Jul 2, 2020
  31. practicalswift deleted the branch on Apr 10, 2021
  32. vijaydasmp referenced this in commit 978426ee1c on Sep 30, 2021
  33. vijaydasmp referenced this in commit 019dc7e2fc on Oct 1, 2021
  34. vijaydasmp referenced this in commit dfa262c93f on Oct 4, 2021
  35. Munkybooty referenced this in commit c6cd5793e4 on Apr 29, 2022
  36. Munkybooty referenced this in commit 9dadb6d216 on Apr 29, 2022
  37. Munkybooty referenced this in commit 65146cea59 on Apr 29, 2022
  38. Munkybooty referenced this in commit 589bdd15df on May 6, 2022
  39. Munkybooty referenced this in commit 72d77d93db on May 17, 2022
  40. Munkybooty referenced this in commit 26ea934aef on May 30, 2022
  41. DrahtBot locked this on Aug 18, 2022

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: 2026-04-14 21:14 UTC

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