Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from Dec 18, 2018

Conversation

Projects
None yet
5 participants
@practicalswift
Copy link
Member

commented Nov 28, 2018

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.

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2018

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

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.

@@ -99,9 +99,9 @@ jobs:
DOCKER_NAME_TAG=ubuntu:16.04
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"
NO_DEPENDS=1
RUN_FUNCTIONAL_TESTS=false # Disabled for now. TODO identify suppressions or exclude specific tests
FUNCTIONAL_TESTS_CONFIG="--exclude feature_block.py,p2p_invalid_messages.py"

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Nov 29, 2018

Member

Why are they excluded?

This comment has been minimized.

Copy link
@practicalswift

practicalswift Dec 3, 2018

Author Member

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.

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Dec 3, 2018

Member

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)

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Dec 3, 2018

Member

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

This comment has been minimized.

Copy link
@practicalswift

practicalswift Dec 3, 2018

Author Member

@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.

.travis.yml Outdated
GOAL="install"
BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER --with-sanitizers=thread --disable-hardening --disable-asm CC=clang CXX=clang++"
BITCOIN_CONFIG="--enable-zmq --disable-wallet --with-gui=qt5 --with-sanitizers=thread --disable-hardening --disable-asm CC=clang CXX=clang++"

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Nov 29, 2018

Member

Why is the wallet and DEBUG_LOCKORDER disabled?

This comment has been minimized.

Copy link
@practicalswift

practicalswift Dec 3, 2018

Author Member

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.

This comment has been minimized.

Copy link
@practicalswift

practicalswift Dec 3, 2018

Author Member

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

This comment has been minimized.

Copy link
@MarcoFalke

MarcoFalke Dec 3, 2018

Member

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 :(

Show resolved Hide resolved test/sanitizer_suppressions/tsan Outdated

@practicalswift practicalswift force-pushed the practicalswift:tsan branch 5 times, most recently Dec 2, 2018

@practicalswift

This comment has been minimized.

Copy link
Member Author

commented Dec 3, 2018

@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.

@practicalswift practicalswift force-pushed the practicalswift:tsan branch Dec 3, 2018

Show resolved Hide resolved .travis/test_06_script.sh Outdated

@DrahtBot DrahtBot removed the Needs rebase label Dec 3, 2018

@practicalswift

This comment has been minimized.

Copy link
Member Author

commented Dec 15, 2018

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

@practicalswift practicalswift force-pushed the practicalswift:tsan branch 2 times, most recently to 5e5138a Dec 17, 2018

@MarcoFalke

This comment has been minimized.

Copy link
Member

commented Dec 17, 2018

utACK 5e5138a

@practicalswift practicalswift force-pushed the practicalswift:tsan branch to eaf4070 Dec 18, 2018

@practicalswift

This comment has been minimized.

Copy link
Member Author

commented Dec 18, 2018

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

@MarcoFalke MarcoFalke merged commit eaf4070 into bitcoin:master Dec 18, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Dec 18, 2018

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

eaf4070 Add suppression for InterruptRPC (fRPCRunning) data race (practicalswift)
5e5138a travis: Use trap and set -e errtrace (Chun Kuan Lee)
069752b build: Enable functional tests in the ThreadSanitizer (TSan) build job (practicalswift)

Pull request description:

  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.

Tree-SHA512: dcc24d311fa124772c3036b16c2bf94732ece36c3e22b4bb8fe941772e52157ab2b1a90b1880b81079c2eef2d344ca7e1da58324b75dbf82d16204d591ad49fb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.