Skip to content

test: Add race:SendZmqMessage tsan suppression #20748

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 1 commit into from
Dec 23, 2020
Merged

test: Add race:SendZmqMessage tsan suppression #20748

maflcko merged 1 commit into from
Dec 23, 2020

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Dec 22, 2020 •
edited
Loading

Add suppression for race:SendZmqMessage, which isn't covered by the existing zmq::* suppression

Fixes #20618

hebasto
hebasto approved these changes Dec 22, 2020
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK fa1d34993f345b2e799baf53174dd258d91f4db1, I have reviewed the code and it looks OK, I agree it can be merged.

@DrahtBot DrahtBot added the Tests label Dec 22, 2020
hebasto
hebasto approved these changes Dec 22, 2020
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

re-ACK fa867daff99bbb2fa2e511f18cac7138e0bd5145

@maflcko
Copy link
Member Author

maflcko commented Dec 22, 2020

restored the zmq::* one because it was working and needed

@hebasto
Copy link
Member

hebasto commented Dec 23, 2020

restored the zmq::* one because it was working and needed

Some related discussions:

From the log it seems possible to replace race:zmq::* with

race:zmq::encoder_base_t
race:zmq::ypipe_t

And one more question: if race:zmq::* does almost all zmq-related job, what is the reason to have race:libzmq?

hebasto
hebasto reviewed Dec 23, 2020
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

re-ACK fa957f8, as my previous comment is not directly related to this pull changes.

@maflcko
Copy link
Member Author

maflcko commented Dec 23, 2020

And one more question: if race:zmq::* does almost all zmq-related job, what is the reason to have race:libzmq?

Maybe it was needed when we still used the libzmq3-dev apt package, and not depends?

@maflcko maflcko merged commit e669c31 into bitcoin:master Dec 23, 2020
@maflcko maflcko deleted the 2012-ciSuppZmq branch December 23, 2020 11:35
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 23, 2020
Merge bitcoin#20748: test: Add race:SendZmqMessage tsan suppression
e61f260 
fa957f8 test: Add race:SendZmqMessage tsan suppression (MarcoFalke)

Pull request description:

  Add suppression for `race:SendZmqMessage`, which isn't covered by the existing `zmq::*` suppression

  Fixes bitcoin#20618

ACKs for top commit:
  hebasto:
    re-ACK fa957f8, as my previous comment is not directly related to this pull changes.

Tree-SHA512: 8642a8b79bbfa4bee89042b66e528f27fd78c5e84a33023df440662e9114e31445fd7b04940f44b11fa4ab7438d346385a21816289c818cce9958a9b16730452
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 13, 2021
@PiRK
test: Add race:SendZmqMessage tsan suppression
38f199e 
Summary:
See https://build.bitcoinabc.org/viewLog.html?buildId=300891&guest=1 for a tsan race while running `interface_zmq.py`
```
 node0 stdout ==================
WARNING: ThreadSanitizer: data race (pid=10367)
  Read of size 8 at 0x7b2400030238 by thread T19:
    #0 memcpy <null> (bitcoind+0x2645a7)
    #1 <null> <null> (libzmq.so.5+0x76220)

  Previous write of size 8 at 0x7b2400030238 by thread T12:
    #0 malloc <null> (bitcoind+0x258ee4)
    #1 <null> <null> (libzmq.so.5+0x39578)
    #2 CZMQAbstractPublishNotifier::SendZmqMessage(char const*, void const*, unsigned long) /work/abc-ci-builds/build-tsan/../../src/zmq/zmqpublishnotifier.cpp:164:14 (bitcoind+0xa1e7a9)
    ...
0
  Location is heap block of size 140 at 0x7b2400030210 allocated by thread T12:
    #0 malloc <null> (bitcoind+0x258ee4)
    #1 <null> <null> (libzmq.so.5+0x39578)
    #2 CZMQAbstractPublishNotifier::SendZmqMessage(char const*, void const*, unsigned long) /work/abc-ci-builds/build-tsan/../../src/zmq/zmqpublishnotifier.cpp:164:14 (bitcoind+0xa1e7a9)
    ...

  Thread T19 'ZMQbg/1' (tid=10388, running) created by main thread at:
    #0 pthread_create <null> (bitcoind+0x25a7ab)
    #1 <null> <null> (libzmq.so.5+0x6e8b3)
    #2 CZMQNotificationInterface::Initialize() /work/abc-ci-builds/build-tsan/../../src/zmq/zmqnotificationinterface.cpp:86:23 (bitcoind+0xa199d4)
    #3 CZMQNotificationInterface::Create() /work/abc-ci-builds/build-tsan/../../src/zmq/zmqnotificationinterface.cpp:61:36 (bitcoind+0xa19328)
    #4 AppInitMain(Config&, RPCServer&, HTTPRPCRequestProcessor&, NodeContext&, interfaces::BlockAndHeaderTipInfo*) /work/abc-ci-builds/build-tsan/../../src/init.cpp:2482:36 (bitcoind+0x30d673)
    #5 AppInit(int, char**) /work/abc-ci-builds/build-tsan/../../src/bitcoind.cpp:181:16 (bitcoind+0x2eaa8e)
    #6 main /work/abc-ci-builds/build-tsan/../../src/bitcoind.cpp:208:13 (bitcoind+0x2eaa8e)

  Thread T12 'b-scheduler' (tid=10380, running) created by main thread at:
    #0 pthread_create <null> (bitcoind+0x25a7ab)
    #1 boost::thread::start_thread_noexcept() <null> (libboost_thread.so.1.67.0+0x1396a)
    #2 boost::thread::thread<std::_Bind<void (* (char const*, std::function<void ()>))(char const*, std::function<void ()>)>&>(std::_Bind<void (* (char const*, std::function<void ()>))(char const*, std::function<void ()>)>&) /usr/include/boost/thread/detail/thread.hpp:266:13 (bitcoind+0x330342)
    #3 boost::thread* boost::thread_group::create_thread<std::_Bind<void (* (char const*, std::function<void ()>))(char const*, std::function<void ()>)> >(std::_Bind<void (* (char const*, std::function<void ()>))(char const*, std::function<void ()>)>) /usr/include/boost/thread/detail/thread_group.hpp:79:60 (bitcoind+0x32039e)
    #4 AppInitMain(Config&, RPCServer&, HTTPRPCRequestProcessor&, NodeContext&, interfaces::BlockAndHeaderTipInfo*) /work/abc-ci-builds/build-tsan/../../src/init.cpp:2243:17 (bitcoind+0x309628)
    #5 AppInit(int, char**) /work/abc-ci-builds/build-tsan/../../src/bitcoind.cpp:181:16 (bitcoind+0x2eaa8e)
    #6 main /work/abc-ci-builds/build-tsan/../../src/bitcoind.cpp:208:13 (bitcoind+0x2eaa8e)

    #1 <null> <null> (libzmq.so.5+0x6e8b3)
    #2 CZMQNotificationInterface::Initialize() /work/abc-ci-builds/build-tsan/../../src/zmq/zmqnotificationinterface.cpp:86:23 (bitcoind+0xa199d4)
    #3 CZMQNotificationInterface::Create() /work/abc-ci-builds/build-tsan/../../src/zmq/zmqnotificationinterface.cpp:61:36 (bitcoind+0xa19328)
    #4 AppInitMain(Config&, RPCServer&, HTTPRPCRequestProcessor&, NodeContext&, interfaces::BlockAndHeaderTipInfo*) /work/abc-ci-builds/build-tsan/../../src/init.cpp:2482:36 (bitcoind+0x30d673)
    #5 AppInit(int, char**) /work/abc-ci-builds/build-tsan/../../src/bitcoind.cpp:181:16 (bitcoind+0x2eaa8e)
```

This is a backport of [[bitcoin/bitcoin#20748 | core#20748]]

Test Plan:
With TSAN:
`for i in $(seq 100); do TSAN_OPTIONS=second_deadlock_stack=1: TSAN_OPTIONS=suppressions=/home/pierre/dev/bitcoin-abc/test/sanitizer_suppressions/tsan test/functional/test_runner.py interface_zmq; done`

This failure is intermittent, but relatively easy to reproduce: I was able to get it with 10 repetitions of the test, prior to adding the suppression.

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10315
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@hebasto hebasto hebasto approved these changes

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Data race in interface_zmq.py

3 participants

@maflcko @hebasto @DrahtBot