Fix warnings when building with -Wthread-safety-analysis #11587

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:–Wthread-safety-analysis changing 1 files +1 −1

practicalswift commented at 7:40 PM on October 31, 2017: contributor

This fix makes the build pass when building with -Werror=thread-safety-analysis.

Prior to this commit:

net_processing.cpp:615:10: error: reading variable 'vExtraTxnForCompact' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
    if (!vExtraTxnForCompact.size())
         ^
net_processing.cpp:616:9: error: reading variable 'vExtraTxnForCompact' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
        vExtraTxnForCompact.resize(max_extra_txn);
        ^
net_processing.cpp:617:5: error: reading variable 'vExtraTxnForCompact' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
    vExtraTxnForCompact[vExtraTxnForCompactIt] = std::make_pair(tx->GetWitnessHash(), tx);
    ^
3 errors generated.
Makefile:5617: recipe for target 'libbitcoin_server_a-net_processing.o' failed

The full -Wthread-safety-analysis machinery (including new annotations) is added in #11226 (>100 commits, awaiting review). This small fix is worth doing just to get the current master branch buildable under -Werror=thread-safety-analysis.

Fix warnings when building with -Wthread-safety-analysis

This fix makes the build pass when building with -Werror=thread-safety-analysis.

Prior to this commit:

```
net_processing.cpp:615:10: error: reading variable 'vExtraTxnForCompact' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
    if (!vExtraTxnForCompact.size())
         ^
net_processing.cpp:616:9: error: reading variable 'vExtraTxnForCompact' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
        vExtraTxnForCompact.resize(max_extra_txn);
        ^
net_processing.cpp:617:5: error: reading variable 'vExtraTxnForCompact' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
    vExtraTxnForCompact[vExtraTxnForCompactIt] = std::make_pair(tx->GetWitnessHash(), tx);
    ^
3 errors generated.
Makefile:5617: recipe for target 'libbitcoin_server_a-net_processing.o' failed
```

The full -Wthread-safety-analysis machinery (including new annotations) is added
in #11226 (>100 commits, awaiting review). This small fix is worth doing just to
get the current master branch buildable under -Werror=thread-safety-analysis.

967e06ad91

TheBlueMatt commented at 8:41 PM on October 31, 2017: member

Isn't this just pulling a single commit out of #10866? #10866 seems more than sufficiently small of a PR for its own review.
practicalswift commented at 9:18 PM on October 31, 2017: contributor

@TheBlueMatt Yes, this one-line-fix is also included in #10866. Since the review of #10866 is taking some time my thinking was that this trivial fix could perhaps get in faster than #10866 which has some additional stuff (Travis CI changes + change of sync.h primitives to std from boost).

Having #10866 reviewed and merged would be even better and would render this PR obsolete :-)
fanquake added the label P2P on Oct 31, 2017
fanquake commented at 10:59 AM on November 1, 2017: member

Agree with @TheBlueMatt. Splitting up PRs to get them "merged quicker" really just creates noise, and fragments discussion. This could be merged, but no-one would really see the benefits until #10866 is merged anyways, so it might as well all happen at once.
fanquake closed this on Nov 1, 2017
practicalswift commented at 12:23 PM on November 1, 2017: contributor

@fanquake I'm not sure I agree with "no-one would really see the benefits until #10866 is merged anyways". I build with -Werror=thread-safety-analysis daily and have to work around this specific issue every day (by patching it manually).
fanquake commented at 12:29 PM on November 1, 2017: member

@practicalswift Lets get #10866 merged then, I'll look at it shortly :)
practicalswift commented at 1:14 PM on November 1, 2017: contributor

@fanquake Thanks! :-)
practicalswift deleted the branch on Apr 10, 2021
DrahtBot locked this on Aug 16, 2022

Contributors

Labels

Linked (view graph)

#11226 [WIP] Add Clang thread safety analysis annotations: GUARDED_BY(lock) / EXCLUSIVE_LOCKS_REQUIRED(lock)