Fix -Wthread-safety-analysis warnings. Compile with -Wthread-safety-analysis if available. #10866

practicalswift · 2017-07-18T14:49:49Z

Add mutex requirement for AddToCompactExtraTransactions(…).
Use -Wthread-safety-analysis if available.
Rebased on top of https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923 - now includes: Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost.

TheBlueMatt · 2017-07-18T20:57:21Z

Do we actually enforce that on Travis? Its nice to have the auto-checker stuff, but AFAIR its (barely) hooked up anywhere?

practicalswift · 2017-07-18T21:24:58Z

@TheBlueMatt Travis is currently not compiling with Clang Thread Safety Analysis warnings (-Wthread-safety-analysis) enabled. (Do we want to change that?)

Personally I build daily using clang with -Wthread-safety-analysis enabled.

In the general case we want to fix all warnings emitted when compiling with -Wthread-safety-analysis, don't we? :-)

Click to expand: Current thread safety warnings

net_processing.cpp:587:10: warning: reading variable 'vExtraTxnForCompact' requires holding mutex 'cs_main' [-Wthread-safety-analysis]
    if (!vExtraTxnForCompact.size())
         ^
net_processing.cpp:60:57: note: Guarded_by declared here.
static std::vector> vExtraTxnForCompact GUARDED_BY(cs_main);
                                                        ^
net_processing.cpp:583:1: note: Thread warning in function 'AddToCompactExtraTransactions'
{
^
net_processing.cpp:588:9: warning: reading variable 'vExtraTxnForCompact' requires holding mutex 'cs_main' [-Wthread-safety-analysis]
        vExtraTxnForCompact.resize(max_extra_txn);
        ^
net_processing.cpp:60:57: note: Guarded_by declared here.
static std::vector> vExtraTxnForCompact GUARDED_BY(cs_main);
                                                        ^
net_processing.cpp:583:1: note: Thread warning in function 'AddToCompactExtraTransactions'
{
^
net_processing.cpp:589:5: warning: reading variable 'vExtraTxnForCompact' requires holding mutex 'cs_main' [-Wthread-safety-analysis]
    vExtraTxnForCompact[vExtraTxnForCompactIt] = std::make_pair(tx->GetWitnessHash(), tx);
    ^
net_processing.cpp:60:57: note: Guarded_by declared here.
static std::vector> vExtraTxnForCompact GUARDED_BY(cs_main);
                                                        ^
net_processing.cpp:583:1: note: Thread warning in function 'AddToCompactExtraTransactions'
{
^

sipa · 2017-07-19T00:39:43Z

If we're going to revive this safety checking infrastructure (it was introduced in #2003, and I don't think updated much since), there are probably many other places to add annotations.

jonasschnelli · 2017-07-19T09:49:55Z

Agree with @sipa.
@practicalswift whats the reason of only add the safety checking infrastructure to AddToCompactExtraTransactions?

practicalswift · 2017-07-19T14:50:24Z

The reason I'm starting with AddToCompactExtraTransactions(…) is simply to fix the only current warning when building with clang's -Wthread-safety-analysis enabled:

net_processing.cpp:587:10: warning: reading variable 'vExtraTxnForCompact' requires holding mutex 'cs_main' [-Wthread-safety-analysis]

The reason for the warning is that vExtraTxnForCompact is marked as guarded by the cs_main-mutex:

static std::vector<std::pair<uint256, CTransactionRef>> vExtraTxnForCompact 
    GUARDED_BY(cs_main);

clang detects that vExtraTxnForCompact is accessed in AddToCompactExtraTransactions(…) and that this function lacks the EXCLUSIVE_LOCKS_REQUIRED(cs_main) annotation that logically should follow from such access (if intentional).

This PR fixes that and after applying this PR the code base is free from -Wthread-safety-analysis warnings.

I'd be glad to extend the usage of GUARDED_BY(…) annotations (and the corresponding EXCLUSIVE_LOCKS_REQUIRED(…) annotations) in the code base, but that is a larger undertaking that probably belongs in a follow-up PR.

I guess one reason not to fix the current warning is if we for some reason instead would want to remove this thread-safety infrastructure. Let me know if that is the case :-)

practicalswift · 2017-07-19T15:06:16Z

Additional reading: The "C/C++ Thread Safety Analysis" paper (Hutchins, Ballman & Sutherland, 2014) paper describing clang thread-safety analysis and how it is used for the Google C++ codebase:

They essentially provide a static type system for threads, and can detect potential race conditions and deadlocks. This paper describes Clang Thread Safety Analysis, a tool which uses annotations to declare and enforce thread safety policies in C and C++ programs.
[…]
It has been deployed on a large scale at Google; all C++ code at Google is now compiled with thread safety analysis enabled by default.
[…]
The GUARDED_BY attribute declares that a thread must lock mu before it can read or write to balance, thus ensuring that the increment and decrement operations are atomic. Similarly,
REQUIRES declares that the calling thread must lock mu before calling withdrawImpl. Because the caller is assumed to have locked mu, it is safe to modify balance within the body of the method.

TheBlueMatt · 2017-07-19T22:12:48Z

Ah! Didn't realize its to fix a warning, yea, lets a) do this and b) can you add a -Werror=thread-safety-analysis here, too? I was informed travis already does a clang build for OSX, so it should be able to catch errors here :).

practicalswift · 2017-07-19T22:26:02Z

@TheBlueMatt Sure! I've updated the PR title to make things more clear :-)

TheBlueMatt · 2017-07-24T15:31:16Z

I think we really should make travis enforce this. It shouldnt be hard cause we already have --enable-werror by default, just need to hook up -Werror=thread-safety-analysis if the compiler supports it (which our clang-osx build should).

practicalswift · 2017-07-24T15:56:49Z

@TheBlueMatt Would something along the lines of the following do the trick for Travis?

diff --git a/.travis.yml b/.travis.yml
index a79428f..423b04e 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -34,7 +34,7 @@ env:
 # No wallet
     - HOST=x86_64-unknown-linux-gnu PACKAGES="python3" DEP_OPTS="NO_WALLET=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports"
 # Cross-Mac
-    - HOST=x86_64-apple-darwin11 PACKAGES="cmake imagemagick libcap-dev librsvg2-bin libz-dev libbz2-dev libtiff-tools python-dev" BITCOIN_CONFIG="--enable-gui --enable-reduce-exports" OSX_SDK=10.11 GOAL="deploy"
+    - HOST=x86_64-apple-darwin11 PACKAGES="cmake imagemagick libcap-dev librsvg2-bin libz-dev libbz2-dev libtiff-tools python-dev" BITCOIN_CONFIG="--enable-gui --enable-reduce-exports" OSX_SDK=10.11 GOAL="deploy" CPPFLAGS="-Werror=thread-safety-analysis"

 before_install:
     - export PATH=$(echo $PATH | tr ':' "\n" | sed '/\/opt\/python/d' | tr "\n" ":" | sed "s|::|:|g")

Or did you mean always enabling that warning if the compiler supports it?

TheBlueMatt · 2017-07-24T20:57:04Z

@practicalswift I was referring to the -Werror support we already have in configure.ac. Best to use that, I'd think.

practicalswift · 2017-07-24T21:57:59Z

@TheBlueMatt Got it! What about this:

diff --git a/configure.ac b/configure.ac
index aea5d71..ec3877d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -241,6 +241,7 @@ if test "x$enable_werror" = "xyes"; then
     AC_MSG_ERROR("enable-werror set but -Werror is not usable")
   fi
   AX_CHECK_COMPILE_FLAG([-Werror=vla],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=vla"],,[[$CXXFLAG_WERROR]])
+  AX_CHECK_COMPILE_FLAG([-Werror=thread-safety-analysis],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=thread-safety-analysis"],,[[$CXXFLAG_WERROR]])
 fi

 if test "x$CXXFLAGS_overridden" = "xno"; then
@@ -249,6 +250,7 @@ if test "x$CXXFLAGS_overridden" = "xno"; then
   AX_CHECK_COMPILE_FLAG([-Wformat],[CXXFLAGS="$CXXFLAGS -Wformat"],,[[$CXXFLAG_WERROR]])
   AX_CHECK_COMPILE_FLAG([-Wvla],[CXXFLAGS="$CXXFLAGS -Wvla"],,[[$CXXFLAG_WERROR]])
   AX_CHECK_COMPILE_FLAG([-Wformat-security],[CXXFLAGS="$CXXFLAGS -Wformat-security"],,[[$CXXFLAG_WERROR]])
+  AX_CHECK_COMPILE_FLAG([-Wthread-safety-analysis],[CXXFLAGS="$CXXFLAGS -Wthread-safety-analysis"],,[[$CXXFLAG_WERROR]])

   ## Some compilers (gcc) ignore unknown -Wno-* options, but warn about all
   ## unknown options if any other warning is produced. Test the -Wfoo case, and

Looks good? :-)

TheBlueMatt · 2017-07-25T20:00:30Z

Heh, I figured you'd incldue it in this PR. Anyway, utACK this pr by itself.

theuni · 2017-07-31T14:18:24Z

utACK ef2aca6c12efc48ce557a87c90ecf2d96ad4885e

TheBlueMatt · 2017-08-01T15:01:57Z

utACK ef2aca6c12efc48ce557a87c90ecf2d96ad4885e, though its useful to hold off merging this until #10923 is ready as a test for #10923.

practicalswift · 2017-08-18T13:50:17Z

Cherry-picked 9df8538 into this PR as suggested by @TheBlueMatt in #10923!

@TheBlueMatt, looks good? :-)

TheBlueMatt · 2017-08-18T21:40:37Z

@ptracticalswift maybe rebase on #10923 and take the patches from https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923 (squashed, of course)?

practicalswift · 2017-08-19T17:07:41Z

@TheBlueMatt Does this PR look good now? Please check my attempt to summarize your changes in the commit message for 695fe31. Let me know if anything should be changed or if you'd rather squash your commits in your branch and have me cherry pick the resulting commit as-is :-)

@TheBlueMatt

a65e028 Build with --enable-werror under OS X (practicalswift) Pull request description: Build with `--enable-werror` under OS X. As suggested by @TheBlueMatt in #10866. This will allow catching violations using Travis CI which does a `clang` build for OS X. Tree-SHA512: 326248897e0776106983e0824e7e80eee3c6e584a1d360f429c30f3375dad83ab4c360c86ab0729bd9ede747ea0caa13cd6a7c35072ed9b845362423c9c37a64

maflcko · 2017-08-23T13:10:55Z

.travis.yml

@@ -34,7 +34,7 @@ env:
 # x86_64 Linux, No wallet
    - HOST=x86_64-unknown-linux-gnu PACKAGES="python3" DEP_OPTS="NO_WALLET=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-glibc-back-compat --enable-reduce-exports"
 # Cross-Mac
-    - HOST=x86_64-apple-darwin11 PACKAGES="cmake imagemagick libcap-dev librsvg2-bin libz-dev libbz2-dev libtiff-tools python-dev" BITCOIN_CONFIG="--enable-gui --enable-reduce-exports" OSX_SDK=10.11 GOAL="deploy"
+    - HOST=x86_64-apple-darwin11 PACKAGES="cmake imagemagick libcap-dev librsvg2-bin libz-dev libbz2-dev libtiff-tools python-dev" BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --enable-werror" OSX_SDK=10.11 GOAL="deploy"


This change is already in master

practicalswift · 2017-11-03T00:12:38Z

@ryanofsky Looks good now? :-)

ryanofsky

Looks good! utACK 1864bea

TheBlueMatt · 2017-11-03T21:17:45Z

src/sync.h

+typedef std::condition_variable CConditionVariable;
+
+/** Just a typedef for std::unique_lock, can be wrapped later if desired */
+typedef std::unique_lock<std::mutex> Lock;


I'd really rather we not call this a "Lock" - its not intended to be the thing used across the codebase as any lock (though we could maybe do so if we add DEBUG_LOCKORDER support here).

You always say what you don't want instead of saying what you do want ;). What would you like instead? If you've seen #11599, the name should make more sense in this context. I think Lock and Mutex are the right names because they match the standard. As you mentioned, you can implement DEBUG_LOCKORDER with them and also do other things useful for debugging, like printing differences in a data structure between time a particular mutex is locked & unlocked.

The original name (CWaitableCriticalSection) was nice and specific - I'm happy to make it more in-line with the more-accepted "lock" terminology, but pointing out that it has only a narrow use-case in our codebase is important.

Ok, but I don't think non-recursive locks should have a narrow usecase in our codebase. I thought everybody agreed non-recursivelocks should be preferred to recursive locks.

Also CWaitableCriticalSection was not the original name for this. There was no original name for this. Previously code just used boost::unique_lock directly.

I was fine with the WaitableCriticalSection (or WaitableLock or whatever) version that was there originally.

@TheBlueMatt @ryanofsky The following diff would work?

diff --git a/src/init.cpp b/src/init.cpp index dd64602..9e3eb6b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -551,7 +551,7 @@ static void BlockNotifyGenesisWait(bool, const CBlockIndex *pBlockIndex) { if (pBlockIndex != nullptr) { { - Lock lock_GenesisWait(cs_GenesisWait); + WaitableLock lock_GenesisWait(cs_GenesisWait); fHaveGenesis = true; } condvar_GenesisWait.notify_all(); @@ -1634,7 +1634,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) // Wait for genesis block to be processed { - Lock lock(cs_GenesisWait); + WaitableLock lock(cs_GenesisWait); while (!fHaveGenesis) { condvar_GenesisWait.wait(lock); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 8fae01b..0ba0e96 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -478,7 +478,7 @@ UniValue getblocktemplate(const JSONRPCRequest& request) { checktxtime = std::chrono::steady_clock::now() + std::chrono::minutes(1); - Lock lock(csBestBlock); + WaitableLock lock(csBestBlock); while (chainActive.Tip()->GetBlockHash() == hashWatchedChain && IsRPCRunning()) { if (cvBlockChange.wait_until(lock, checktxtime) == std::cv_status::timeout) diff --git a/src/sync.h b/src/sync.h index 66b6d96..20556af 100644 --- a/src/sync.h +++ b/src/sync.h @@ -105,7 +105,7 @@ typedef AnnotatedMixin<std::mutex> CWaitableCriticalSection; typedef std::condition_variable CConditionVariable; /** Just a typedef for std::unique_lock, can be wrapped later if desired */ -typedef std::unique_lock<std::mutex> Lock; +typedef std::unique_lock<std::mutex> WaitableLock; #ifdef DEBUG_LOCKCONTENTION void PrintLockContention(const char* pszName, const char* pszFile, int nLine);

@TheBlueMatt @ryanofsky The following diff would work?

Yes for me. (As I mentioned in my last comment I am happy with any naming in this pr since #11599 can clean up any inconsistencies.)

@ryanofsky Excellent! OK with you @TheBlueMatt? :-)

Yea, looks good, thanks!

@TheBlueMatt

…o std from boost. Commit 1. This code was written by @TheBlueMatt in the following branch: * https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923 This commit message was written by me (@practicalswift) who also squashed @TheBlueMatt's commits into one and tried to summarize the changes made. Commit 2. Remove boost include. Remove boost mentions in comments.

…alysis if --enable-werror)

The vector `vExtraTxnForCompact`, which is guarded by the mutex `cs_main`, is accessed in `AddToCompactExtraTransactions(…)`.

practicalswift · 2017-11-06T16:43:38Z

@ryanofsky @TheBlueMatt Name change patch applied. Squashed and pushed. Could you please re-review? :-)

TheBlueMatt · 2017-11-06T16:52:19Z

utACK 76ea17c

ryanofsky

utACK 76ea17c, confirmed only change since last review is name change.

sipa · 2017-11-07T18:20:49Z

utACK 76ea17c. Nice to see some further unboostification.

…hread-safety-analysis if available. 76ea17c Add mutex requirement for AddToCompactExtraTransactions(…) (practicalswift) 4616c82 Use -Wthread-safety-analysis if available (+ -Werror=thread-safety-analysis if --enable-werror) (practicalswift) 7e319d6 Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost. (Matt Corallo) Pull request description: * Add mutex requirement for `AddToCompactExtraTransactions(…)`. * Use `-Wthread-safety-analysis` if available. * Rebased on top of https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923 - now includes: Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost. Tree-SHA512: fb7365f85daa2741c276a1c899228181a8d46af51db7fbbdffceeaff121a3eb2ab74d7c8bf5e7de879bcc5042d00d24cb4649c312d51caba45a3f6135fd8b38f

…ith -Wthread-safety-analysis if available. 76ea17c Add mutex requirement for AddToCompactExtraTransactions(…) (practicalswift) 4616c82 Use -Wthread-safety-analysis if available (+ -Werror=thread-safety-analysis if --enable-werror) (practicalswift) 7e319d6 Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost. (Matt Corallo) Pull request description: * Add mutex requirement for `AddToCompactExtraTransactions(…)`. * Use `-Wthread-safety-analysis` if available. * Rebased on top of https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923 - now includes: Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost. Tree-SHA512: fb7365f85daa2741c276a1c899228181a8d46af51db7fbbdffceeaff121a3eb2ab74d7c8bf5e7de879bcc5042d00d24cb4649c312d51caba45a3f6135fd8b38f

…se of uninitialized memory 870f0cd build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory (practicalswift) Pull request description: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory. First UBSan, then ASan followed by TSan... and now: yes, the wait is over -- **MSan is finally here!** :) Some historical context: * 2017: Continuous compilation with Clang Thread Safety analysis enabled (#10866, #10923) * 2018: Continuous testing with trapping on signed integer overflows (`-ftrapv`) (#12686) * 2018: Continuous testing of use of locale dependent functions (#13041) * 2018: Continuous testing of format strings (#13705) * 2018: Continuous compilation with MSVC `TreatWarningAsError` (#14151) * 2018: Continuous testing under UndefinedBehaviorSanitizer – UBSan (#14252, #14673, #17006) * 2018: Continuous testing under AddressSanitizer – ASan (#14794, #17205, #17674) * 2018: Continuous testing under ThreadSanitizer – TSan (#14829) * 2019: Continuous testing in an unsigned char environment (`-funsigned-char`) (#15134) * 2019: Continuous compile-time testing of assumptions we're making (#15391) * 2019: Continuous testing of fuzz test cases under Valgrind (#17633, #18159, #18166) * 2020: Finally... MemorySanitizer – MSAN! :) What is the next step? What tools should we add to CI to keep bugs from entering `master`? :) ACKs for top commit: MarcoFalke: ACK 870f0cd Tree-SHA512: 38327c8b75679d97d469fe42e704cacd1217447a5a603701dd8a58ee50b3be2c10248f8d68a479ed081c0c4b254589d3081c9183f991640b06ef689061f75578

…ith -Wthread-safety-analysis if available. 76ea17c Add mutex requirement for AddToCompactExtraTransactions(…) (practicalswift) 4616c82 Use -Wthread-safety-analysis if available (+ -Werror=thread-safety-analysis if --enable-werror) (practicalswift) 7e319d6 Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost. (Matt Corallo) Pull request description: * Add mutex requirement for `AddToCompactExtraTransactions(…)`. * Use `-Wthread-safety-analysis` if available. * Rebased on top of https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923 - now includes: Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost. Tree-SHA512: fb7365f85daa2741c276a1c899228181a8d46af51db7fbbdffceeaff121a3eb2ab74d7c8bf5e7de879bcc5042d00d24cb4649c312d51caba45a3f6135fd8b38f

…etect use of uninitialized memory 870f0cd build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory (practicalswift) Pull request description: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory. First UBSan, then ASan followed by TSan... and now: yes, the wait is over -- **MSan is finally here!** :) Some historical context: * 2017: Continuous compilation with Clang Thread Safety analysis enabled (bitcoin#10866, bitcoin#10923) * 2018: Continuous testing with trapping on signed integer overflows (`-ftrapv`) (bitcoin#12686) * 2018: Continuous testing of use of locale dependent functions (bitcoin#13041) * 2018: Continuous testing of format strings (bitcoin#13705) * 2018: Continuous compilation with MSVC `TreatWarningAsError` (bitcoin#14151) * 2018: Continuous testing under UndefinedBehaviorSanitizer – UBSan (bitcoin#14252, bitcoin#14673, bitcoin#17006) * 2018: Continuous testing under AddressSanitizer – ASan (bitcoin#14794, bitcoin#17205, bitcoin#17674) * 2018: Continuous testing under ThreadSanitizer – TSan (bitcoin#14829) * 2019: Continuous testing in an unsigned char environment (`-funsigned-char`) (bitcoin#15134) * 2019: Continuous compile-time testing of assumptions we're making (bitcoin#15391) * 2019: Continuous testing of fuzz test cases under Valgrind (bitcoin#17633, bitcoin#18159, bitcoin#18166) * 2020: Finally... MemorySanitizer – MSAN! :) What is the next step? What tools should we add to CI to keep bugs from entering `master`? :) ACKs for top commit: MarcoFalke: ACK 870f0cd Tree-SHA512: 38327c8b75679d97d469fe42e704cacd1217447a5a603701dd8a58ee50b3be2c10248f8d68a479ed081c0c4b254589d3081c9183f991640b06ef689061f75578

…ith -Wthread-safety-analysis if available. 76ea17c Add mutex requirement for AddToCompactExtraTransactions(…) (practicalswift) 4616c82 Use -Wthread-safety-analysis if available (+ -Werror=thread-safety-analysis if --enable-werror) (practicalswift) 7e319d6 Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost. (Matt Corallo) Pull request description: * Add mutex requirement for `AddToCompactExtraTransactions(…)`. * Use `-Wthread-safety-analysis` if available. * Rebased on top of https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923 - now includes: Fix -Wthread-safety-analysis warnings. Change the sync.h primitives to std from boost. Tree-SHA512: fb7365f85daa2741c276a1c899228181a8d46af51db7fbbdffceeaff121a3eb2ab74d7c8bf5e7de879bcc5042d00d24cb4649c312d51caba45a3f6135fd8b38f

jonasschnelli added the Refactoring label Jul 19, 2017

practicalswift changed the title ~~Add mutex requirement for AddToCompactExtraTransactions(…)~~ Fix -Wthread-safety-analysis warning: Add mutex requirement for AddToCompactExtraTransactions(…) Jul 19, 2017

practicalswift mentioned this pull request Jul 25, 2017

travis: Build with --enable-werror under OS X #10923

Merged

practicalswift force-pushed the lock-requirement-for-AddToCompactExtraTransactions branch from ef2aca6 to c6d9b1a Compare August 18, 2017 13:49

practicalswift changed the title ~~Fix -Wthread-safety-analysis warning: Add mutex requirement for AddToCompactExtraTransactions(…)~~ Fix -Wthread-safety-analysis warning: Add mutex requirement for AddToCompactExtraTransactions(…) + enable --enable-werror for OS X Travis Aug 18, 2017

practicalswift force-pushed the lock-requirement-for-AddToCompactExtraTransactions branch from fe63a42 to 473e056 Compare August 19, 2017 14:24

practicalswift force-pushed the lock-requirement-for-AddToCompactExtraTransactions branch 2 times, most recently from a165b4b to eeee7c1 Compare August 19, 2017 17:01

maflcko reviewed Aug 23, 2017

View reviewed changes

practicalswift force-pushed the lock-requirement-for-AddToCompactExtraTransactions branch from 2fcb082 to 1864bea Compare November 3, 2017 00:13

ryanofsky reviewed Nov 3, 2017

View reviewed changes

ryanofsky mentioned this pull request Nov 3, 2017

scripted-diff: Small locking rename #11599

Merged

TheBlueMatt reviewed Nov 3, 2017

View reviewed changes

TheBlueMatt and others added 3 commits November 6, 2017 17:41

Use -Wthread-safety-analysis if available (+ -Werror=thread-safety-an…

4616c82

…alysis if --enable-werror)

Add mutex requirement for AddToCompactExtraTransactions(…)

76ea17c

The vector `vExtraTxnForCompact`, which is guarded by the mutex `cs_main`, is accessed in `AddToCompactExtraTransactions(…)`.

practicalswift force-pushed the lock-requirement-for-AddToCompactExtraTransactions branch from 1864bea to 76ea17c Compare November 6, 2017 16:42

ryanofsky reviewed Nov 6, 2017

View reviewed changes

sipa merged commit 76ea17c into bitcoin:master Nov 7, 2017

practicalswift mentioned this pull request Apr 30, 2018

Meta-issue: Add Clang thread safety analysis annotations #13129

Closed

30 tasks

practicalswift mentioned this pull request Mar 7, 2020

build: Add MemorySanitizer (MSan) in Travis to detect use of uninitialized memory #18288

Merged

hebasto mentioned this pull request May 7, 2020

Thread Safety Analysis warnings #18636

Closed

LarryRuane mentioned this pull request Mar 26, 2021

Allow zcashd to be built cleanly with -Wthread-safety zcash/zcash#5035

Open

practicalswift deleted the lock-requirement-for-AddToCompactExtraTransactions branch April 10, 2021 19:33

str4d mentioned this pull request Apr 12, 2021

Sync backports zcash/zcash#5057

Merged

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

Fix -Wthread-safety-analysis warnings. Compile with -Wthread-safety-analysis if available. #10866

Fix -Wthread-safety-analysis warnings. Compile with -Wthread-safety-analysis if available. #10866

Uh oh!

Conversation

practicalswift commented Jul 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Jul 18, 2017

Uh oh!

practicalswift commented Jul 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sipa commented Jul 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli commented Jul 19, 2017

Uh oh!

practicalswift commented Jul 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Jul 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Jul 19, 2017

Uh oh!

practicalswift commented Jul 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Jul 24, 2017

Uh oh!

practicalswift commented Jul 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Jul 24, 2017

Uh oh!

practicalswift commented Jul 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Jul 25, 2017

Uh oh!

theuni commented Jul 31, 2017

Uh oh!

TheBlueMatt commented Aug 1, 2017

Uh oh!

practicalswift commented Aug 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TheBlueMatt commented Aug 18, 2017

Uh oh!

practicalswift commented Aug 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Nov 3, 2017

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanofsky Nov 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

practicalswift commented Jul 18, 2017 •

edited

Loading

practicalswift commented Jul 18, 2017 •

edited

Loading

sipa commented Jul 19, 2017 •

edited

Loading

practicalswift commented Jul 19, 2017 •

edited

Loading

practicalswift commented Jul 19, 2017 •

edited

Loading

practicalswift commented Jul 19, 2017 •

edited

Loading

practicalswift commented Jul 24, 2017 •

edited

Loading

practicalswift commented Jul 24, 2017 •

edited

Loading

practicalswift commented Aug 18, 2017 •

edited

Loading

practicalswift commented Aug 19, 2017 •

edited

Loading

ryanofsky Nov 3, 2017 •

edited

Loading