Add missing cs_main locks when accessing chainActive

practicalswift commented at 5:50 pm on November 2, 2017: contributor

Add missing cs_main locks when accessing chainActive. (The variable chainActive is guarded by the mutex cs_main.)
Add corresponding annotations (GUARDED_BY + EXCLUSIVE_LOCKS_REQUIRED).

in src/validation.cpp:2673 in 52b2f34e58 outdated

2668@@ -2669,8 +2669,11 @@ static bool ReceivedBlockTransactions(const CBlock &block, CValidationState& sta
2669                 LOCK(cs_nBlockSequenceId);
2670                 pindex->nSequenceId = nBlockSequenceId++;
2671             }
2672-            if (chainActive.Tip() == nullptr || !setBlockIndexCandidates.value_comp()(pindex, chainActive.Tip())) {
2673-                setBlockIndexCandidates.insert(pindex);
2674+            {
2675+                LOCK(cs_main);

sdaftuar commented at 5:53 pm on November 2, 2017:

I think we can just add an AssertLockHeld(cs_main) to the top of this function – this function only has two callers, both of which lock cs_main before calling.

EDIT: actually one of the callers does AssertLockHeld(cs_main).

in src/wallet/wallet.cpp:2656 in 52b2f34e58 outdated

2650@@ -2648,7 +2651,10 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
2651     if (GetRandInt(10) == 0)
2652         txNew.nLockTime = std::max(0, (int)txNew.nLockTime - GetRandInt(100));
2653 
2654-    assert(txNew.nLockTime <= (unsigned int)chainActive.Height());
2655+    {
2656+        LOCK(cs_main);
2657+        assert(txNew.nLockTime <= (unsigned int)chainActive.Height());

morcos commented at 6:02 pm on November 2, 2017:

I think we should just get rid of this assert. We can’t guarantee chainActive height will not go down.

practicalswift force-pushed on Nov 2, 2017

practicalswift commented at 9:10 pm on November 2, 2017: contributor

@sdaftuar @morcos Thanks for reviewing. Suggested changes incorporated! :-)

fanquake added the label Refactoring on Nov 3, 2017

in src/wallet/wallet.cpp:2740 in 1c93531dec outdated

2638@@ -2639,7 +2639,10 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
2639     // enough, that fee sniping isn't a problem yet, but by implementing a fix
2640     // now we ensure code won't be written that makes assumptions about
2641     // nLockTime that preclude a fix later.
2642-    txNew.nLockTime = chainActive.Height();
2643+    {
2644+        LOCK(cs_main);

promag commented at 10:02 pm on November 3, 2017:

Remove, all callers have cs_main locked, assert lock is held instead. Ping @ryanofsky as he says the goal is to removed recursive locks.

practicalswift commented at 10:37 am on November 6, 2017:

@promag Thanks! Now fixed.

practicalswift commented at 3:04 pm on November 6, 2017:

@promag I’m getting an assertion failure when running test/functional/test_runner.py with your suggested patch applied. Does it pass for you? Reverting to previous version for now.

promag commented at 3:11 pm on November 6, 2017:

I’ll check.

practicalswift commented at 7:09 am on March 20, 2018:

@promag See @TheBlueMatt’s comment about cs_main locking in CWallet::CreateTransaction(…): #11596 (review)

promag commented at 10:05 pm on November 3, 2017: member

utACK.

practicalswift force-pushed on Nov 6, 2017

promag commented at 4:19 pm on November 6, 2017: member

@practicalswift following #11596 (review), I believe a7eb21a is the fix.

practicalswift force-pushed on Nov 6, 2017

practicalswift commented at 4:38 pm on November 6, 2017: contributor

@promag What is the reasoning behind the patch (a7eb21a6f64cc042e695f9f483e49a5adaed7c4e) – what are the underlying locking requirements (exact variables being guarded)? Your patch is likely correct, but I’m trying to understand exactly why :-)

in src/rest.cpp:515 in fdea0e21da outdated

509@@ -510,7 +510,10 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
510         // serialize data
511         // use exact same output as mentioned in Bip64
512         CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION);
513-        ssGetUTXOResponse << chainActive.Height() << chainActive.Tip()->GetBlockHash() << bitmap << outs;
514+        {
515+            LOCK(cs_main);
516+            ssGetUTXOResponse << chainActive.Height() << chainActive.Tip()->GetBlockHash() << bitmap << outs;

TheBlueMatt commented at 7:46 pm on November 6, 2017:

nit: care to do the serialization of bitmap/outs outside of cs_main (same below)?

in src/wallet/wallet.cpp:2597 in fdea0e21da outdated

2593@@ -2594,6 +2594,8 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
2594 bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet,
2595                                 int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign)
2596 {
2597+    AssertLockHeld(cs_main);

TheBlueMatt commented at 8:09 pm on November 6, 2017:

Ugh, can we not add more cs_main requirements - if the goal is to resolve the chainActive.Height() issue only then please add the cs_main around just that, its not gonna hurt anything to not hold cs_main for the duration of this function, and we really should be trying to remove cs_main from this function, not go the other way.

practicalswift force-pushed on Nov 6, 2017

practicalswift commented at 8:48 pm on November 6, 2017: contributor

@TheBlueMatt Thanks for reviewing. Feedback addressed.

in src/wallet/rpcwallet.cpp:3019 in cd8ac3cc86 outdated

3014@@ -3015,8 +3015,11 @@ UniValue fundrawtransaction(const JSONRPCRequest& request)
3015     CAmount nFeeOut;
3016     std::string strFailReason;
3017 
3018-    if (!pwallet->FundTransaction(tx, nFeeOut, changePosition, strFailReason, lockUnspents, setSubtractFeeFromOutputs, coinControl)) {
3019-        throw JSONRPCError(RPC_WALLET_ERROR, strFailReason);
3020+    {
3021+        LOCK2(cs_main, pwallet->cs_wallet);

TheBlueMatt commented at 4:51 pm on November 7, 2017:

Please no. Can we instead add the locking in CreateTransaction itself? We need to be pushing locks down, especially when it comes to cs_main, not putting more locks in RPC functions.

practicalswift force-pushed on Nov 7, 2017

practicalswift commented at 6:09 pm on November 7, 2017: contributor

Reverting the patch suggested by @promag. The locking is now made down in CreateTransaction as suggested by @TheBlueMatt.

TheBlueMatt commented at 6:25 pm on November 7, 2017: member

utACK 2e441c91e7806e6ae781ae0b0aefea958079d15b Thanks!

in src/rest.cpp:515 in 2e441c91e7 outdated

509@@ -510,7 +510,11 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
510         // serialize data
511         // use exact same output as mentioned in Bip64
512         CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION);
513-        ssGetUTXOResponse << chainActive.Height() << chainActive.Tip()->GetBlockHash() << bitmap << outs;
514+        {
515+            LOCK(cs_main);
516+            ssGetUTXOResponse << chainActive.Height() << chainActive.Tip()->GetBlockHash();

luke-jr commented at 4:22 pm on November 10, 2017:

This isn’t sufficient I think. We don’t want to allow chainActive to change from when we get the results, to when we identify the chain they’re for. So extract this info above in the first lock…

promag commented at 4:49 pm on November 10, 2017:

Which first lock?

practicalswift commented at 5:31 pm on November 10, 2017:

@luke-jr Thanks for reviewing. Do you mean like this?

 0diff --git a/src/rest.cpp b/src/rest.cpp
 1index 0ed11e6..649059e 100644
 2--- a/src/rest.cpp
 3+++ b/src/rest.cpp
 4@@ -479,6 +479,8 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
 5     std::string bitmapStringRepresentation;
 6     std::vector<bool> hits;
 7     bitmap.resize((vOutPoints.size() + 7) / 8);
 8+    int chainActiveHeight;
 9+    uint256 chainActiveTipBlockHash;
10     {
11         LOCK2(cs_main, mempool.cs);
12
13@@ -503,6 +505,9 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
14             bitmapStringRepresentation.append(hit ? "1" : "0"); // form a binary string representation (human-readable for json output)
15             bitmap[i / 8] |= ((uint8_t)hit) << (i % 8);
16         }
17+
18+        chainActiveHeight = chainActive.Height();
19+        chainActiveTipBlockHash = chainActive.Tip()->GetBlockHash();
20     }
21
22     switch (rf) {
23@@ -510,11 +515,7 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
24         // serialize data
25         // use exact same output as mentioned in Bip64
26         CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION);
27-        {
28-            LOCK(cs_main);
29-            ssGetUTXOResponse << chainActive.Height() << chainActive.Tip()->GetBlockHash();
30-        }
31-        ssGetUTXOResponse << bitmap << outs;
32+        ssGetUTXOResponse << chainActiveHeight << chainActiveTipBlockHash << bitmap << outs;
33         std::string ssGetUTXOResponseString = ssGetUTXOResponse.str();
34
35         req->WriteHeader("Content-Type", "application/octet-stream");
36@@ -524,11 +525,7 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
37
38     case RF_HEX: {
39         CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION);
40-        {
41-            LOCK(cs_main);
42-            ssGetUTXOResponse << chainActive.Height() << chainActive.Tip()->GetBlockHash();
43-        }
44-        ssGetUTXOResponse << bitmap << outs;
45+        ssGetUTXOResponse << chainActiveHeight << chainActiveTipBlockHash << bitmap << outs;
46         std::string strHex = HexStr(ssGetUTXOResponse.begin(), ssGetUTXOResponse.end()) + "\n";
47
48         req->WriteHeader("Content-Type", "text/plain");
49@@ -541,11 +538,8 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
50
51         // pack in some essentials
52         // use more or less the same output as mentioned in Bip64
53-        {
54-            LOCK(cs_main);
55-            objGetUTXOResponse.push_back(Pair("chainHeight", chainActive.Height()));
56-            objGetUTXOResponse.push_back(Pair("chaintipHash", chainActive.Tip()->GetBlockHash().GetHex()));
57-        }
58+        objGetUTXOResponse.push_back(Pair("chainHeight", chainActiveHeight));
59+        objGetUTXOResponse.push_back(Pair("chaintipHash", chainActiveTipBlockHash.GetHex()));
60         objGetUTXOResponse.push_back(Pair("bitmap", bitmapStringRepresentation));
61
62         UniValue utxos(UniValue::VARR);

in src/rest.cpp:529 in 2e441c91e7 outdated

523@@ -520,7 +524,11 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
524 
525     case RF_HEX: {
526         CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION);
527-        ssGetUTXOResponse << chainActive.Height() << chainActive.Tip()->GetBlockHash() << bitmap << outs;
528+        {
529+            LOCK(cs_main);
530+            ssGetUTXOResponse << chainActive.Height() << chainActive.Tip()->GetBlockHash();

luke-jr commented at 4:22 pm on November 10, 2017:

Same

in src/rest.cpp:547 in 2e441c91e7 outdated

540@@ -533,8 +541,11 @@ static bool rest_getutxos(HTTPRequest* req, const std::string& strURIPart)
541 
542         // pack in some essentials
543         // use more or less the same output as mentioned in Bip64
544-        objGetUTXOResponse.push_back(Pair("chainHeight", chainActive.Height()));
545-        objGetUTXOResponse.push_back(Pair("chaintipHash", chainActive.Tip()->GetBlockHash().GetHex()));
546+        {
547+            LOCK(cs_main);
548+            objGetUTXOResponse.push_back(Pair("chainHeight", chainActive.Height()));
549+            objGetUTXOResponse.push_back(Pair("chaintipHash", chainActive.Tip()->GetBlockHash().GetHex()));

luke-jr commented at 4:23 pm on November 10, 2017:

Same.

practicalswift force-pushed on Nov 10, 2017

practicalswift commented at 7:05 pm on November 10, 2017: contributor

Added commit 9b3d094894350933342b5e352785cfdd9f2d03fe addressing @luke-jr:s feedback.

Please re-review :-)

TheBlueMatt commented at 9:10 pm on November 10, 2017: member

@practicalswift why did you rebase this? It makes reviewing something that was previously reviewed much more difficult. Mind squashing the two commits? There seems to be no reason to have them separate.

luke-jr approved

luke-jr commented at 8:34 am on November 11, 2017: member

utACK

luke-jr referenced this in commit b7982dea93 on Nov 11, 2017

luke-jr referenced this in commit 95606a0b3a on Nov 11, 2017

practicalswift force-pushed on Nov 12, 2017

practicalswift commented at 3:44 pm on November 12, 2017: contributor

Now squashed into one commit. Please re-review! :-) @TheBlueMatt The reason for the rebase was that I wanted to get rebased on top of 76ea17c7964c15dd90e10c2c257cdeb5847b3d69 (IIRC) in order to get the build to pass – I always compile with -Werror=thread-safety-analysis :-)

The reason for the two commits was to allow separate review for the latter commit in the case that I had misunderstood @luke-jr:s suggestion and needed to revert.

practicalswift force-pushed on Nov 23, 2017

practicalswift commented at 12:24 pm on November 23, 2017: contributor

Updated:

Added a few more missing cs_main locks.
Added a commit with the Clang thread safety analysis annotation chainActive GUARDED_BY(cs_main) and the corresponding EXCLUSIVE_LOCKS_REQUIRED(…) annotations that follow from that.

Please review :-)

practicalswift force-pushed on Nov 23, 2017

TheBlueMatt commented at 2:49 pm on November 25, 2017: member

The addition of annotations here isn’t complete - do you not need to add a EXCLUSIVE_LOCKS_REQUIRED annotation to the declarations, not the definitions of functions?

On November 23, 2017 4:24:23 AM PST, practicalswift notifications@github.com wrote:

Updated:

Added a few more missing cs_main locks.

Added a commit with the Clang thread safety analysis annotations chainActive GUARDED_BY(cs_main) and the corresponding EXCLUSIVE_LOCKS_REQUIRED(…) annotations.

Please review :-)

– You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/bitcoin/bitcoin/pull/11596#issuecomment-346604992

practicalswift force-pushed on Nov 29, 2017

practicalswift commented at 10:10 pm on November 29, 2017: contributor

I’ve now pushed an updated version adding a missing OpenWallets() lock and some additional annotations. @TheBlueMatt - to make the annotations 100 % complete and moved to the declarations we’ll have to handle some header files for which the existence of cs_main is currently not known. More specifically the following header files:

net_processing.h
qt/transactiondesc.h
qt/transactionrecord.h
rpc/blockchain.h
wallet/init.h
wallet/wallet.h

What is the most appropriate way to handle cs_main in the respective cases above? I guess we don’t want to sprinkle extern CCriticalSection cs_main; all over the place? :-)

TheBlueMatt commented at 5:52 pm on December 4, 2017: member

@practicalswift No, we aren’t adding the annotations just to add the annotations. In order for them to be really effective and provide the kind of guarantees we want they need to be listed everywhere the function which needs the annotation is declared, not just ate the definition. I’d rather we leave them out than add them just to function declarations. The annotations are going to be highly infective for things like cs_main - they’re gonna propagate backwards through the callstack pretty deep, but that’s ok, its how it should be. We shouldn’t avoid it, just gotta take the dive (though preferably not in this PR, please, lets just get this one merged).

practicalswift commented at 7:14 pm on December 4, 2017: contributor

@TheBlueMatt Is that an utACK for this PR as-is? :-)

TheBlueMatt commented at 7:15 pm on December 4, 2017: member

If you remove the annotation commit to revert to where people had previously reviewed this, I’m happy to review again.

practicalswift force-pushed on Dec 4, 2017

practicalswift commented at 7:21 pm on December 4, 2017: contributor

@TheBlueMatt Reverted and pushed. Now only keeping the lock addition (removed adding annotations). Looks good? :-)

in src/init.cpp:1605 in f03db4d345 outdated

1581@@ -1581,8 +1582,11 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
1582 
1583     // ********************************************************* Step 8: load wallet
1584 #ifdef ENABLE_WALLET
1585-    if (!OpenWallets())
1586-        return false;
1587+    {
1588+        LOCK(cs_main);
1589+        if (!OpenWallets())

TheBlueMatt commented at 4:03 pm on December 8, 2017:

I’d kinda prefer we push this lock down into wallet/init. Seemd weird to export the cs_main requirement to OpenWallets.

in src/init.cpp:1513 in f03db4d345 outdated

1491@@ -1492,6 +1492,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
1492                         strLoadError = _("Error initializing block database");
1493                         break;
1494                     }
1495+                    LOCK(cs_main);

TheBlueMatt commented at 4:03 pm on December 8, 2017:

There are a ton of cs_main-required things in this block, seems weird to put this all the way down here. For static-analysis purposes, its probably best to just take cs_main at the top of the try{} block and hold it through the whole chain-load process.

practicalswift force-pushed on Feb 22, 2018

practicalswift commented at 8:52 pm on February 22, 2018: contributor

@TheBlueMatt Thanks for reviewing! Feedback address. Would you mind re-reviewing? :-)

in src/init.cpp:1634 in f122e02eaf outdated

1631@@ -1634,10 +1632,13 @@ bool AppInitMain()
1632 
1633     // Either install a handler to notify us when genesis activates, or set fHaveGenesis directly.
1634     // No locking, as this happens before any background thread is started.

luke-jr commented at 3:50 pm on March 1, 2018:

This comment needs to be updated at least. Preferably with an explanation why you’re adding a lock here…

luke-jr approved

luke-jr commented at 3:53 pm on March 1, 2018: member

utACK

practicalswift commented at 7:24 am on March 2, 2018: contributor

@luke-jr Good point! Comment updated. Please re-review :-)

MarcoFalke commented at 5:22 pm on March 2, 2018: member

Since https://github.com/bitcoin/bitcoin/pull/11226/files#diff-349fbb003d5ae550a2e8fa658e475880R447 is closed, could you amend the second commit to include the clang annotation?

practicalswift force-pushed on Mar 12, 2018

practicalswift commented at 8:12 pm on March 12, 2018: contributor

@MarcoFalke Good idea! Annotations added. Please review :-)

practicalswift force-pushed on Mar 13, 2018

luke-jr commented at 9:06 pm on March 13, 2018: member

Why did you un-remove the double-lock at “The block database contains a block which appears to be from the future.”?

practicalswift commented at 9:15 pm on March 13, 2018: contributor

@luke-jr I’m actually investigating a dead-lock right now which seems to have been introduced during the last rebase. Which double-lock are you referring to? Do you have a diff?

I’m investigating as we speak.

practicalswift commented at 9:34 pm on March 13, 2018: contributor

@luke-jr You mean this one?

 0diff --git a/src/init.cpp b/src/init.cpp
 1index e7544e8..f18a947 100644
 2--- a/src/init.cpp
 3+++ b/src/init.cpp
 4@@ -1429,7 +1429,6 @@ bool AppInitMain()
 5         nStart = GetTimeMillis();
 6         do {
 7             try {
 8-                LOCK(cs_main);
 9                 UnloadBlockIndex();
10                 pcoinsTip.reset();
11                 pcoinsdbview.reset();

practicalswift renamed this:
~~Add missing cs_main locks when accessing chainActive~~
Add missing cs_main locks when accessing chainActive [wip]
on Mar 13, 2018

luke-jr commented at 3:01 am on March 14, 2018: member

Search init.cpp for “The block database contains a block which appears to be from the future.”. It grabs cs_main immediately before that. But cs_main is already held here now.

practicalswift force-pushed on Mar 19, 2018

practicalswift commented at 8:37 pm on March 19, 2018: contributor

I’ve now reworked this PR and split it up in three commits:

Add missing LOCK(cs_main):s where required for chainActive access
Annotation: Add chainActive GUARDED_BY(cs_main) – does not change run-time behaviour
Annotation: Add EXCLUSIVE_LOCKS_REQUIRED(...) as implied by the chainActive guard – does not change run-time behaviour

Please re-review :-)

/cc @sdaftuar @morcos @promag @TheBlueMatt @luke-jr @MarcoFalke

practicalswift renamed this:
~~Add missing cs_main locks when accessing chainActive [wip]~~
Add missing cs_main locks when accessing chainActive
on Mar 19, 2018

practicalswift force-pushed on Mar 20, 2018

practicalswift referenced this in commit 6855c57dfc on Mar 20, 2018

practicalswift force-pushed on Mar 23, 2018

practicalswift referenced this in commit 889f4e568f on Mar 23, 2018

practicalswift force-pushed on Apr 3, 2018

practicalswift referenced this in commit 2210a4cda5 on Apr 3, 2018

practicalswift commented at 11:39 am on April 3, 2018: contributor

Rebased!

practicalswift force-pushed on Apr 9, 2018

practicalswift referenced this in commit cdb29ceeea on Apr 9, 2018

practicalswift commented at 1:13 pm on April 9, 2018: contributor

Rebased!

practicalswift force-pushed on Apr 9, 2018

practicalswift force-pushed on Apr 14, 2018

practicalswift referenced this in commit 604b60f686 on Apr 14, 2018

practicalswift commented at 1:14 pm on April 14, 2018: contributor

Rebased! Please re-review :-)

in src/wallet/test/wallet_tests.cpp:317 in f882f9160e outdated

311@@ -301,10 +312,16 @@ class ListCoinsTestingSetup : public TestChain100Setup
312             blocktx = CMutableTransaction(*wallet->mapWallet.at(tx->GetHash()).tx);
313         }
314         CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
315-        LOCK(wallet->cs_wallet);
316-        auto it = wallet->mapWallet.find(tx->GetHash());
317-        BOOST_CHECK(it != wallet->mapWallet.end());
318-        it->second.SetMerkleBranch(chainActive.Tip(), 1);
319+        std::map<uint256, CWalletTx>::iterator it;
320+        {

TheBlueMatt commented at 6:12 pm on April 17, 2018:

Can you just take both locks instead of adding a new scope and lots of diff lines? Its in a test, locks shouldn’t matter much.

in src/interfaces/wallet.cpp:60 in f882f9160e outdated

53@@ -54,7 +54,7 @@ class PendingWalletTxImpl : public PendingWalletTx
54 };
55 
56 //! Construct wallet tx struct.
57-WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
58+WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_main)

TheBlueMatt commented at 6:14 pm on April 17, 2018:

a) annotations need to go in the header file, not the cpp file. b) I dont think we can put locks required in the interfaces file, as it may become a process boundary. The callee will have to take the lock itself.

ryanofsky commented at 6:41 pm on April 17, 2018:

annotations need to go in the header file

This is ok, these are just internal functions (in an anonymous namespace) not shared or exposed in headers.

TheBlueMatt commented at 6:15 pm on April 17, 2018: member

I guess this is somewhat duplicative with #11652, no? My review comments there apply here too.

Locking annotations: Add missing cs_main locks (when accessing chainActive) d4409961cb

Remove invalid assertion: We can't guarantee chainActive height will not go down

As noted by @marcos in https://github.com/bitcoin/bitcoin/pull/11596#discussion_r148614708

77b7b06d7f

Locking annotations: Add EXCLUSIVE_LOCKS_REQUIRED(...) annotations that follow from "chainActive GUARDED_BY(cs_main)" 5ff77fb571

practicalswift force-pushed on Apr 26, 2018

Locking annotations: Variable chainActive is guarded by cs_main b0279a0293

practicalswift force-pushed on Apr 26, 2018

practicalswift commented at 4:22 am on April 26, 2018: contributor

Rebased and updated. Please re-review :-)

practicalswift commented at 4:37 pm on April 30, 2018: contributor

Closing. Will add new PR based on #11652 and #13083.

practicalswift closed this on Apr 30, 2018

practicalswift deleted the branch on Apr 10, 2021

DrahtBot locked this on Aug 16, 2022

Add missing cs_main locks when accessing chainActive #11596