Push cs_main locks down in ProcessBlock #4148

1. 
   sipa commented at 8:57 pm on May 7, 2014: member

   For now, built on top of #3884.
2. 
   laanwj commented at 8:30 am on May 8, 2014: member

   Fixes at least #3966.
3. laanwj added this to the milestone 0.10.0 on May 8, 2014
4. in src/main.cpp: in ed85993dc1 outdated

   2188+
   2189+bool ActivateBestChain(CValidationState &state) {
   2190+    CBlockIndex *pindexNewTip = NULL;
   2191+    CBlockIndex *pindexMostWork = NULL;
   2192+    do {
   2193+        bool fInitialDownload;
   

   ---

   ---

   

   laanwj commented at 8:33 pm on May 8, 2014:

   Should we have a boost::this_thread::interruption_point() here?

   ---

   

   sipa commented at 9:39 pm on May 8, 2014:

   Good idea!
5. 
   sipa commented at 10:26 pm on May 8, 2014: member

   Rebased and added an interruption point in ActivateBestChain, as suggested by @laanwj.
6. 
   laanwj commented at 1:22 pm on May 19, 2014: member

   ACK on code changes Also I’ve run with this patch for about a week now on my node. No problems.
7. 
   sipa commented at 1:30 pm on May 19, 2014: member

   I’m running this patch together with #2784, #4100 and #4177, and I’m seeing occasional segfaults. I believe this patch is the most likely cause, so I’ll investigate first.
8. 
   jgarzik commented at 1:46 pm on May 19, 2014: contributor

   initial review ACK (untested).

   I will try some lock-debug testing later this week.

9. in src/main.cpp: in f6a6adcc43 outdated

   2225+            uint256 hashNewTip = pindexNewTip->GetBlockHash();
   2226+            // Relay inventory, but don't relay old inventory during initial block download.
   2227+            int nBlockEstimate = Checkpoints::GetTotalBlocksEstimate();
   2228+            LOCK(cs_vNodes);
   2229+            BOOST_FOREACH(CNode* pnode, vNodes)
   2230+                if (chainActive.Height() > (pnode->nStartingHeight != -1 ? pnode->nStartingHeight - 2000 : nBlockEstimate))
   

   ---

   ---

   

   cozz commented at 6:24 pm on May 19, 2014:

   Isn’t chainActive here and a few lines below accessed without cs_main?

   ---

   

   sipa commented at 11:37 am on May 22, 2014:

   Nice catch. However, chainActive should never be modified anywhere but in the thread calling this method (the message handler), so it can’t be the cause for the segfault i observed (perhaps #4204 fixed it, though). I’ll fix it.
10. 
    sipa commented at 10:25 am on May 23, 2014: member

    @cozz Updated.
11. 
    sipa commented at 10:27 am on May 23, 2014: member

    I haven’t seen any segfaults anymore since running on top of #4204.
12. 
    laanwj commented at 10:52 am on May 23, 2014: member

    I still haven’t had any segfaults running with this.
13. Get rid of the static chainMostWork (optimization) 77339e5aec
14. Allow ActivateBestChain to release its lock on cs_main 4e0eed88ac
15. Move all post-chaintip-change notifications to ActivateBestChain 202e01941c
16. Push cs_mains down in ProcessBlock 18e72167dd
17. 
    BitcoinPullTester commented at 0:38 am on June 9, 2014: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/18e72167ddfeaea95253b62994c6d64b55b35005 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
18. 
    laanwj commented at 8:11 am on June 9, 2014: member

    ACK
19. laanwj merged this on Jun 9, 2014
20. laanwj closed this on Jun 9, 2014

    ---

21. laanwj referenced this in commit 07b233a1b6 on Jun 9, 2014
22. in src/main.cpp: in 18e72167dd

    2663@@ -2649,7 +2664,11 @@ bool ProcessBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBl
    2664         mapOrphanBlocksByPrev.erase(hashPrev);
    2665     }
    2666 
    2667-    LogPrintf("ProcessBlock: ACCEPTED\n");
    

    ---

    ---

    

    rebroad commented at 1:59 pm on June 20, 2014:

    Ahem! #3984 did this!
23. DrahtBot locked this on Sep 8, 2021

Contributors
sipa laanwj jgarzik cozz BitcoinPullTester rebroad

Milestone
0.10.0