Skip to content

net: Fix CThreadInterrupt::sleep_for TSan issues #19029

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
Closed
hebasto wants to merge 2 commits into from
Closed

net: Fix CThreadInterrupt::sleep_for TSan issues #19029

hebasto wants to merge 2 commits into from

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented May 20, 2020

This PR adds to the ThreadDNSAddressSeed and ThreadOpenAddedConnections threads their own CThreadInterrupt objects, and fixes ThreadSanitizer's "double lock of a mutex" warnings.

Fix #19024

@hebasto hebasto mentioned this pull request May 20, 2020
Closed
maflcko
maflcko reviewed May 20, 2020
View reviewed changes
@hebasto hebasto force-pushed the 200520-interrupts branch from 7bb5707 to a46122c Compare May 20, 2020 16:45
@hebasto
Copy link
Member Author

hebasto commented May 20, 2020

Updated 7bb5707 -> a46122c (pr19029.01 -> pr19029.02, diff):

Looks like a member to me, no?

@DrahtBot DrahtBot added the P2P label May 20, 2020
@DrahtBot
Copy link
Contributor

DrahtBot commented May 20, 2020 •
edited
Loading

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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.

This was referenced May 20, 2020
Closed
Merged
Closed
@hebasto
Copy link
Member Author

hebasto commented May 21, 2020 •
edited
Loading

I'm not sure these warnings aren't false positive due to the Fedora clang's bug. See: #19024 (comment)

So closing for now.

@hebasto hebasto closed this May 21, 2020
@maflcko
Copy link
Member

maflcko commented May 21, 2020

An alternative would be to make the mutex recursive for now

@hebasto hebasto reopened this May 21, 2020
@hebasto
Copy link
Member Author

hebasto commented May 21, 2020

An alternative would be to make the mutex recursive for now

This won't compile, unfortunately.

@DrahtBot DrahtBot mentioned this pull request May 28, 2020
Merged
@hebasto hebasto force-pushed the 200520-interrupts branch from a46122c to bad0f26 Compare May 31, 2020 06:39
@hebasto
Copy link
Member Author

hebasto commented May 31, 2020

Rebased a46122c -> bad0f26 (pr19029.02 -> pr19029.03) due to the conflict with #16939.

@hebasto
Copy link
Member Author

hebasto commented May 31, 2020

Closing for now. See: #19024 (comment)

@hebasto hebasto closed this May 31, 2020
@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

1 more reviewer

@maflcko maflcko maflcko left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

P2P

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

UB: double lock of mutex in init.cpp

3 participants

@hebasto @DrahtBot @maflcko