Do not hide compile-time thread safety warnings #19668

hebasto · 2020-08-05T12:43:36Z

On the way of transit from RecursiveMutex to Mutex (see #19303) it is crucial to have run-time AssertLockHeld() assertion that does not hide compile-time Clang Thread Safety Analysis warnings.

On master (65e4eca) using AssertLockHeld() could hide Clang Thread Safety Analysis warnings, e.g., with the following patch applied:

--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -607,7 +607,7 @@ public:
     void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
 
     void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs);
-    void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main);
+    void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
     void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs);
     void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs);

Clang compiles the code without any thread safety warnings.

See "Add missed thread safety annotations" commit for the actual thread safety warnings that are fixed in this PR.

hebasto · 2020-08-05T12:44:10Z

Friendly ping @ajtowns @vasild

maflcko

weak Concept ACK. Will review some more later

src/net.h

hebasto · 2020-08-05T14:38:28Z

Updated fa91579 -> 3f00515 (pr19668.01 -> pr19668.02, diff).

This is an alternative approach without issues mentioned by @MarcoFalke:

It seems a layer-violation to import the validation lock cs_main into the net.h header to annotate this function. Also, this approach doesn't seem like it scales, since it has to be done for each mutex that is held during a call to ForEachNode.

hebasto · 2020-08-05T14:42:04Z

@MarcoFalke

weak Concept ACK. Will review some more later

Updated motivation in the OP.

ajtowns · 2020-08-05T17:01:48Z

Concept ACK.

I'd suggest going further and annotating the AssertLock(Not)Held calls with EXCLUSIVE_LOCKS_REQUIRED(cs) (or !cs) too. This catches existing missing annotations in validation.cpp:CheckSequenceLocks and wallet/scriptpubkeyman.cpp:DescriptorScriptPubKeyMan::AddDescriptKeyWithDb so seems worthwhile. Patch at https://github.com/ajtowns/bitcoin/commits/202008-assertlockheld

hebasto · 2020-08-05T17:30:43Z

@ajtowns

I'd suggest going further and annotating the AssertLock(Not)Held calls with EXCLUSIVE_LOCKS_REQUIRED(cs) (or !cs) too. This catches existing missing annotations in validation.cpp:CheckSequenceLocks and wallet/scriptpubkeyman.cpp:DescriptorScriptPubKeyMan::AddDescriptKeyWithDb so seems worthwhile. Patch at https://github.com/ajtowns/bitcoin/commits/202008-assertlockheld

Thanks! Done.

ajtowns · 2020-08-06T06:01:27Z

Might be good to fix up the lock annotations before removing the ASSERT annotation on AssertLockHeldInternal so you can compile successfully at each commit? Otherwise looks good to me.

hebasto · 2020-08-06T06:07:13Z

@ajtowns

Might be good to fix up the lock annotations before removing the ASSERT annotation on AssertLockHeldInternal so you can compile successfully at each commit? Otherwise looks good to me.

Done, commits are reordered now.

jnewbery · 2020-08-07T10:56:00Z

Concept ACK. Perhaps the Locking/mutex usage notes section of developer-notes.md should be updated with the current best practice for annotating/asserting locks.

hebasto · 2020-08-07T11:18:00Z

@jnewbery

Concept ACK. Perhaps the Locking/mutex usage notes section of developer-notes.md should be updated with the current best practice for annotating/asserting locks.

You read my mind :)

Do you want me adding a doc commit to this PR?

jnewbery · 2020-08-07T12:09:02Z

Do you want me adding a doc commit to this PR?

I think it's fine to add it to this PR.

src/sync.h

hebasto · 2020-08-08T10:05:46Z

Do you want me adding a doc commit to this PR?

I think it's fine to add it to this PR.

Done.

@jnewbery @jonatack Mind checking wording in the doc commit?

vasild

ACK 7666823

vasild · 2020-08-10T11:56:53Z

doc/developer-notes.md

+{
+public:
+    ...
+    mutable RecursiveMutex cs;


nit: Maybe use an example that does not involve RecursiveMutex since we recommend against using it and if we manage to obliterate it from the code, then this example will require adjusting.

Oh, I've failed to find an example in our code base that embraces a Mutex instance, an annotation in function member declaration, and an assertion in function member definition.

vasild · 2020-08-10T12:01:35Z

This PR basically replaces the attribute ASSERT_EXCLUSIVE_LOCK() of AssertLockHeldInternal() with EXCLUSIVE_LOCKS_REQUIRED() and fixes some of the exposed warnings.

It is unfortunate that we can't have both attributes.

jnewbery · 2020-08-13T11:35:22Z

This all seems reasonable and the updated developer docs are clear and make sense.

@ajtowns : what does your 👎 mean here #19668 (comment)? Are you saying that we can have both attributes, or that it's not unfortunate, or that we can't have both and that this PR is going in the wrong direction? As someone who doesn't know very much about the safety annotations, it's difficult for me to interpret what your objection is.

DrahtBot · 2020-08-20T20:58:53Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

p2p: Unify Send and Receive protocol versions #17785 (p2p: Unify Send and Receive protocol versions 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.

hebasto · 2020-08-24T17:32:41Z

Rebased 7666823 -> 9ee4653 (pr19668.05 -> pr19668.06) due to the conflict with #19704.

maflcko

It seems to be doing the exact opposite of what the clang docs say, so it would be good to explain why this needs to be different

src/net_processing.cpp

ea74e10 doc: Add best practice for annotating/asserting locks (Hennadii Stepanov) 2ee7743 sync.h: Make runtime lock checks require compile-time lock checks (Anthony Towns) 23d71d1 Do not hide compile-time thread safety warnings (Hennadii Stepanov) 3ddc150 Add missed thread safety annotations (Hennadii Stepanov) af9ea55 Use LockAssertion utility class instead of AssertLockHeld() (Hennadii Stepanov) Pull request description: On the way of transit from `RecursiveMutex` to `Mutex` (see bitcoin#19303) it is crucial to have run-time `AssertLockHeld()` assertion that does _not_ hide compile-time Clang Thread Safety Analysis warnings. On master (65e4eca) using `AssertLockHeld()` could hide Clang Thread Safety Analysis warnings, e.g., with the following patch applied: ```diff --- a/src/txmempool.h +++ b/src/txmempool.h @@ -607,7 +607,7 @@ public: void addUnchecked(const CTxMemPoolEntry& entry, setEntries& setAncestors, bool validFeeEstimate = true) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); void removeRecursive(const CTransaction& tx, MemPoolRemovalReason reason) EXCLUSIVE_LOCKS_REQUIRED(cs); - void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs, cs_main); + void removeForReorg(const CCoinsViewCache* pcoins, unsigned int nMemPoolHeight, int flags) EXCLUSIVE_LOCKS_REQUIRED(cs_main); void removeConflicts(const CTransaction& tx) EXCLUSIVE_LOCKS_REQUIRED(cs); void removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); ``` Clang compiles the code without any thread safety warnings. See "Add missed thread safety annotations" commit for the actual thread safety warnings that are fixed in this PR. ACKs for top commit: MarcoFalke: ACK ea74e10 🎙 jnewbery: ACK ea74e10 ajtowns: ACK ea74e10 Tree-SHA512: 8cba996e526751a1cb0e613c0cc1b10f027a3e9945fbfb4bd30f6355fd36b9f9c2e1e95ed3183fc254b42df7c30223278e18e5bdb5e1ef85db7fef067595d447

ryanofsky · 2020-09-03T17:25:08Z

src/net_processing.cpp

@@ -1327,7 +1326,7 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std:
    }

    m_connman.ForEachNode([this, &pcmpctblock, pindex, &msgMaker, fWitnessEnabled, &hashBlock](CNode* pnode) {
-        AssertLockHeld(cs_main);
+        LockAssertion lock(::cs_main);


What's the reason to prefer LockAssertion to AssertLockHeld if AssertLockHeld works at compile time and runtime and LockAssertion only works at compile time? Maybe this could be clarified https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#threads-and-synchronization

When TSA fails to detect that a mutex is locked we do want to use LockAssertion as using AssertLockHeld no longer helps:

$ git diff diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 0e049bd66..90bd626fc 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1370,7 +1370,7 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std: } m_connman.ForEachNode([this, &pcmpctblock, pindex, &msgMaker, fWitnessEnabled, &hashBlock](CNode* pnode) { - LockAssertion lock(::cs_main); + AssertLockHeld(::cs_main); // TODO: Avoid the repeated-serialization here if (pnode->nVersion < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect) $ make net_processing.cpp:1373:9: warning: calling function 'AssertLockHeldInternal<AnnotatedMixin<std::recursive_mutex> >' requires holding mutex 'cs_main' exclusively [-Wthread-safety-analysis] AssertLockHeld(::cs_main); ^ ./sync.h:79:28: note: expanded from macro 'AssertLockHeld' #define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs) ^ net_processing.cpp:1378:9: warning: calling function 'ProcessBlockAvailability' requires holding mutex 'cs_main' exclusively [-Wthread-safety-analysis] ProcessBlockAvailability(pnode->GetId()); ^ net_processing.cpp:1379:30: warning: calling function 'State' requires holding mutex 'cs_main' exclusively [-Wthread-safety-analysis] CNodeState &state = *State(pnode->GetId()); ^ net_processing.cpp:1383:18: warning: calling function 'PeerHasHeader' requires holding mutex 'cs_main' exclusively [-Wthread-safety-analysis] !PeerHasHeader(&state, pindex) && PeerHasHeader(&state, pindex->pprev)) { ^ net_processing.cpp:1383:51: warning: calling function 'PeerHasHeader' requires holding mutex 'cs_main' exclusively [-Wthread-safety-analysis] !PeerHasHeader(&state, pindex) && PeerHasHeader(&state, pindex->pprev)) { ^ 5 warnings generated.

And this LockAssertion application is documented:

bitcoin/src/sync.h

Lines 355 to 357 in a0a422c

// Utility class for indicating to compiler thread analysis that a mutex is

// locked (when it couldn't be determined otherwise).

struct SCOPED_LOCKABLE LockAssertion

Maybe this could be clarified https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#threads-and-synchronization

Mind suggesting the clarification as my English is so poor?

Oh, I wasn't aware AssertLockHeld annotations changed from ASSERT_EXCLUSIVE_LOCK
to EXCLUSIVE_LOCKS_REQUIRED in #19668. This seems like a questionable decision to me, because now instead of being able to use AssertLockHeld everywhere, we have separate confusing AssertLockHeld and LockAssertion calls.

I think I would want to clarify this by changing AssertLockHeld annotations back to ASSERT_EXCLUSIVE_LOCK, making it a weaker assertion that only causes errors at runtime, never compile time. Then we could drop LockAssertion and s/LockAssertion/AssertLockHeld/ and only have one type of assertion to think about.

If this doesn't work because it interferes with work you are doing, another option
might be to s/LockAssertion/WeaklyAssertLockHeld/ everywhere. (Also maybe simplify the implementation to be an ASSERT_EXCLUSIVE_LOCK function instead of a class instance that requires a variable declaration.) The new naming should make it clearer which call is preferred:

AssertLockHeld - stricter assertion which causes errors at compile time if the compiler supports annotations, or causes errors at runtime if the compiler doesn't support annotations

WeaklyAssertLockHeld - less strict alternative which only triggers errors at runtime, never compile-time, and can be used when it's not possible for compiler to determine that the mutex is locked.

no longer needlessly disables the compile time checks

It's funny the only reason I ever called it was to disable compile time checks, and now it doesn't do that anymore! 😄 If really the only thing AssertLockHeld is doing is adding runtime checks on top of compile time checks that are already present, I think that is nearly useless. Anyway, can take it up in another PR.

Anyway, can take it up in another PR.

Created #19865 to experiment

You use LockAssertion g(cs_foo); to tell the compiler that a lock is guaranteed to already be held within a code block even though the function annotations don't guarantee it. AssertLockHeld is useful if you're not compiling with clang.

AssertLockHeld is useful if you're not compiling with clang.

Yes, that's what I mean by "nearly useless." It can help developers who aren't using clang, and who are compiling with DEBUG_LOCKORDER, and who want to wait for an assert to trigger at runtime. But it won't make the codebase more thread safe because we already don't merge changes that don't compile with clang.

Any case, if you see problems with #19865, it would be good to comment there. I'm not an expert on lock annotations, so maybe the AssertLockHeld calls removed in #19865 are actually doing something useful besides haphazardly repeating compile time assertions at runtime.

AssertLockHeld is useful if you're not compiling with clang.

Yes, that's what I mean by "nearly useless." It can help developers who aren't using clang, and who are compiling with DEBUG_LOCKORDER, and who want to wait for an assert to trigger at runtime. But it won't make the codebase more thread safe because we already don't merge changes that don't compile with clang.

I disagree with statements about "useless" AssertLockHeld.

…ertion -BEGIN VERIFY SCRIPT- git grep -l AssertLockHeldInternal src | xargs sed -i /AssertLockHeldInternal/s/EXCLUSIVE_LOCKS_REQUIRED/ASSERT_EXCLUSIVE_LOCK/ git grep -l AssertLockHeld src | xargs sed -i '/^ *AssertLockHeld(.*);/d' git grep -l LockAssertion src | xargs sed -i 's/LockAssertion lock($.*$);/AssertLockHeld(\1);/g' -END VERIFY SCRIPT-

020f051 refactor: CTxMemPool::IsUnbroadcastTx() requires CTxMemPool::cs lock (Hennadii Stepanov) 7c4bd03 refactor: CTxMemPool::GetTotalTxSize() requires CTxMemPool::cs lock (Hennadii Stepanov) fa5fcb0 refactor: CTxMemPool::ClearPrioritisation() requires CTxMemPool::cs lock (Hennadii Stepanov) 7140b31 refactor: CTxMemPool::ApplyDelta() requires CTxMemPool::cs lock (Hennadii Stepanov) 66e47e5 refactor: CTxMemPool::UpdateChild() requires CTxMemPool::cs lock (Hennadii Stepanov) 9398077 refactor: CTxMemPool::UpdateParent() requires CTxMemPool::cs lock (Hennadii Stepanov) Pull request description: This is another step to transit `CTxMemPool::cs` from `RecursiveMutex` to `Mutex`. Split out from #19306. Only trivial thread safety annotations and lock assertions added. No new locks. No behavior change. Refactoring `const uint256` to `const uint256&` was [requested](#19647 (comment)) by **promag**. Please note that now, since #19668 has been merged, it is safe to apply `AssertLockHeld()` macros as they do not swallow compile time Thread Safety Analysis warnings. ACKs for top commit: promag: Core review ACK 020f051. jnewbery: Code review ACK 020f051 vasild: ACK 020f051 Tree-SHA512: a31e389142d5a19b25fef0aaf1072a337278564528b5cc9209df88ae548a31440e1b8dd9bae0169fd7aa59ea06e22fe5e0413955386512b83ef1f3e7d941e890

…le cases 020f051 refactor: CTxMemPool::IsUnbroadcastTx() requires CTxMemPool::cs lock (Hennadii Stepanov) 7c4bd03 refactor: CTxMemPool::GetTotalTxSize() requires CTxMemPool::cs lock (Hennadii Stepanov) fa5fcb0 refactor: CTxMemPool::ClearPrioritisation() requires CTxMemPool::cs lock (Hennadii Stepanov) 7140b31 refactor: CTxMemPool::ApplyDelta() requires CTxMemPool::cs lock (Hennadii Stepanov) 66e47e5 refactor: CTxMemPool::UpdateChild() requires CTxMemPool::cs lock (Hennadii Stepanov) 9398077 refactor: CTxMemPool::UpdateParent() requires CTxMemPool::cs lock (Hennadii Stepanov) Pull request description: This is another step to transit `CTxMemPool::cs` from `RecursiveMutex` to `Mutex`. Split out from bitcoin#19306. Only trivial thread safety annotations and lock assertions added. No new locks. No behavior change. Refactoring `const uint256` to `const uint256&` was [requested](bitcoin#19647 (comment)) by **promag**. Please note that now, since bitcoin#19668 has been merged, it is safe to apply `AssertLockHeld()` macros as they do not swallow compile time Thread Safety Analysis warnings. ACKs for top commit: promag: Core review ACK 020f051. jnewbery: Code review ACK 020f051 vasild: ACK 020f051 Tree-SHA512: a31e389142d5a19b25fef0aaf1072a337278564528b5cc9209df88ae548a31440e1b8dd9bae0169fd7aa59ea06e22fe5e0413955386512b83ef1f3e7d941e890

…ertion -BEGIN VERIFY SCRIPT- git grep -l AssertLockHeldInternal src | xargs sed -i /AssertLockHeldInternal/s/EXCLUSIVE_LOCKS_REQUIRED/ASSERT_EXCLUSIVE_LOCK/ git grep -l AssertLockHeld src | xargs sed -i '/^ *AssertLockHeld(.*);/d' git grep -l LockAssertion src | xargs sed -i 's/LockAssertion lock($.*$);/AssertLockHeld(\1);/g' -END VERIFY SCRIPT-

…kAssertion -BEGIN VERIFY SCRIPT- git grep -l AssertLockHeldInternal | xargs sed -i /AssertLockHeldInternal/s/EXCLUSIVE_LOCKS_REQUIRED/ASSERT_EXCLUSIVE_LOCK/ git grep -l AssertLockHeld ':!src/sync.h' | xargs sed -i '/^ *AssertLockHeld(.*);/d' git grep -l LockAssertion | xargs sed -i 's/LockAssertion lock($.*$);/AssertLockHeld(\1);/g' -END VERIFY SCRIPT-

Summary: This change prepares for upcoming commit "Do not hide compile-time thread safety warnings" by replacing AssertLockHeld() with LockAssertion() where needed. This is a backport of [[bitcoin/bitcoin#19668 | core#19668]] [1/5] bitcoin/bitcoin@af9ea55 Backport notse: - We do not have a lock in RelayTransaction because we did not backport [[bitcoin/bitcoin#18044 | core#18044]] (commit ac88e2eb619821ad7ae1d45d4b40be69051d3999) - This change is partially reverted in D10172 after D10171 makes it possible to annotate the lambda functions with `EXCLUSIVE_LOCKS_REQUIRED` Test Plan: With TSAN: `ninja check check-functional` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10161

Summary: This is needed for upcoming commit "sync.h: Make runtime lock checks require compile-time lock checks" to pass. This is a backport of [[bitcoin/bitcoin#19668 | core#19668]] [2/5] bitcoin/bitcoin@3ddc150 Depends on D10161 Test Plan: With TSAN: `ninja check check-functional` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10162

Summary: This is a backport of [[bitcoin/bitcoin#19668 | core#19668]] [3/5] bitcoin/bitcoin@23d71d1 Depends on D10162 Test Plan: With TSAN: `ninja check check-functional` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10163

Summary: This is a backport of [[bitcoin/bitcoin#19668 | core#19668]] [4/5] bitcoin/bitcoin@2ee7743 Depends on D10163 Test Plan: With TSAN: `ninja check check-functional` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10164

Summary: Backport note: I removed the paragraph about `LockAssertion`, because the code example causes a false positive linter error, and this paragraph would anyway get removed anyway in a few commits (D10173) This is a backport of [[bitcoin/bitcoin#19668 | core#19668]] [5/5] bitcoin/bitcoin@ea74e10 Test Plan: proofreading Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10165

laanwj added the Build system label Aug 5, 2020

maflcko reviewed Aug 5, 2020

View reviewed changes

src/net.h Outdated Show resolved Hide resolved

hebasto force-pushed the 200805-assert branch from fa91579 to 3f00515 Compare August 5, 2020 14:33

hebasto force-pushed the 200805-assert branch from fa50315 to fcf6569 Compare August 6, 2020 06:06

This was referenced Aug 6, 2020

Deduplicate parent txid loop of requested transactions and missing parents of orphan transactions #19596

Merged

refactor: Keep mempool interface in validation #19629

Closed

vasild reviewed Aug 7, 2020

View reviewed changes

src/sync.h Show resolved Hide resolved

vasild approved these changes Aug 10, 2020

View reviewed changes

vasild mentioned this pull request Aug 11, 2020

Add thread safety annotations to CTxMemPool methods #19647

Closed

DrahtBot mentioned this pull request Aug 20, 2020

Net processing: move ProcessMessage() to PeerLogicValidation #19704

Merged

DrahtBot mentioned this pull request Aug 21, 2020

p2p: Unify Send and Receive protocol versions #17785

Merged

DrahtBot added the Needs rebase label Aug 24, 2020

hebasto force-pushed the 200805-assert branch from 7666823 to 9ee4653 Compare August 24, 2020 17:30

DrahtBot removed the Needs rebase label Aug 24, 2020

maflcko reviewed Aug 29, 2020

View reviewed changes

src/net_processing.cpp Show resolved Hide resolved

hebasto deleted the 200805-assert branch September 1, 2020 07:03

hebasto mentioned this pull request Sep 1, 2020

Avoid locking CTxMemPool::cs recursively in simple cases #19854

Merged

ryanofsky reviewed Sep 3, 2020

View reviewed changes

ryanofsky mentioned this pull request Sep 3, 2020

scripted-diff: Restore AssertLockHeld after #19668, remove LockAssertion #19865

Closed

This was referenced Sep 4, 2020

Switch BlockMap to use an unordered_set under the hood #19677

Closed

Remove mempool global #19556

Merged

Tidy up ProcessOrphanTx #19498

Merged

Add fee_est tool for debugging fee estimation code #10443

Closed

eBPF Linux tracepoints #19866

Merged

This was referenced Sep 4, 2020

Avoid locking CTxMemPool::cs recursively in some cases #19872

Closed

Avoid locking CTxMemPool::cs recursively in CTxMemPool::DynamicMemoryUsage() #19901

Closed

sync: Replace LockAssertion with AssertLockHeldUnverified #19918

Closed

ryanofsky mentioned this pull request Sep 10, 2020

sync: use proper TSA attributes #19929

Closed

This was referenced Sep 19, 2020

Replace LockAssertion with AssertLockHeld, remove LockAssertion #19979

Merged

sync.h: fix LockAssertion error reporting #19970

Closed

Remove dead CheckForkWarningConditionsOnNewFork #19905

Merged

refactor: Some wallet cleanups #19980

Merged

hebasto mentioned this pull request Apr 5, 2021

refactor: Remove negative lock annotations from globals #21598

Merged

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

	// Utility class for indicating to compiler thread analysis that a mutex is
	// locked (when it couldn't be determined otherwise).
	struct SCOPED_LOCKABLE LockAssertion

Do not hide compile-time thread safety warnings #19668

Do not hide compile-time thread safety warnings #19668

Uh oh!

Conversation

hebasto commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Aug 5, 2020

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hebasto commented Aug 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Aug 5, 2020

Uh oh!

ajtowns commented Aug 5, 2020

Uh oh!

hebasto commented Aug 5, 2020

Uh oh!

ajtowns commented Aug 6, 2020

Uh oh!

hebasto commented Aug 6, 2020

Uh oh!

jnewbery commented Aug 7, 2020

Uh oh!

hebasto commented Aug 7, 2020

Uh oh!

jnewbery commented Aug 7, 2020

Uh oh!

Uh oh!

hebasto commented Aug 8, 2020

Uh oh!

vasild 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!

vasild commented Aug 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jnewbery commented Aug 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Aug 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

hebasto commented Aug 24, 2020

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

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

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!

Uh oh!

hebasto commented Aug 5, 2020 •

edited

Loading

hebasto commented Aug 5, 2020 •

edited

Loading

vasild commented Aug 10, 2020 •

edited

Loading

jnewbery commented Aug 13, 2020 •

edited

Loading

DrahtBot commented Aug 20, 2020 •

edited

Loading