Avoid permanent cs_main lock in getblockheader #12153

1. 
   promag commented at 2:26 PM on January 11, 2018: member

   This PR reduces the cs_main lock scope in getblockheader RPC.

2. promag force-pushed on Jan 11, 2018
3. fanquake added the label RPC/REST/ZMQ on Jan 11, 2018
4. promag force-pushed on Jan 11, 2018
5. promag force-pushed on Jan 12, 2018
6. promag force-pushed on Jan 12, 2018
7. promag force-pushed on Jan 15, 2018
8. 
   ryanofsky commented at 10:33 PM on February 5, 2018: contributor

   utACK last commit only 5e924f4d22a16b0c46d4c6cb19877892331d41fa (previous commits are from base PR)

9. 
   jnewbery commented at 9:56 PM on April 3, 2018: member

   needs rebase (but probably not worth doing until #12151 is merged)

10. MarcoFalke added the label Needs rebase on Jun 6, 2018
11. promag force-pushed on Jun 10, 2018
12. promag force-pushed on Jun 10, 2018
13. DrahtBot removed the label Needs rebase on Jun 11, 2018
14. DrahtBot cross-referenced this on Jun 14, 2018 from issue Consistently validate txid / blockhash length and encoding in rpc calls by Empact
15. DrahtBot cross-referenced this on Jul 6, 2018 from issue Trivial: delete "to" in comment by YutakaNakasone
16. DrahtBot closed this on Aug 2, 2018

    ---

17. DrahtBot reopened this on Aug 2, 2018

    ---

18. DrahtBot cross-referenced this on Aug 10, 2018 from issue Track best-possible-headers (TheBlueMatt) by Sjors
19. DrahtBot cross-referenced this on Sep 10, 2018 from issue [RPC] decodeblock by instagibbs
20. DrahtBot cross-referenced this on Sep 21, 2018 from issue rpc: Remove cs_main lock from blockToJSON and blockheaderToJSON by promag
21. DrahtBot added the label Needs rebase on Sep 24, 2018
22. DrahtBot added the label Up for grabs on Jan 3, 2019
23. DrahtBot closed this on Jan 3, 2019

    ---

24. fanquake reopened this on Jan 4, 2019

    ---

25. fanquake removed the label Up for grabs on Jan 4, 2019
26. promag force-pushed on Jan 4, 2019
27. 
    promag commented at 12:35 PM on January 4, 2019: member

    Rebased.

28. fanquake removed the label Needs rebase on Jan 4, 2019
29. bitcoin deleted a comment on Jan 4, 2019
30. bitcoin deleted a comment on Jan 4, 2019
31. bitcoin deleted a comment on Jan 4, 2019
32. bitcoin deleted a comment on Jan 4, 2019
33. in src/rpc/blockchain.cpp:761 in 4572a3901f outdated

     761 | +    const CBlockIndex* pblockindex;
     762 | +    const CBlockIndex* tip;
     763 | +    {
     764 | +        LOCK(cs_main);
     765 | +        pblockindex = LookupBlockIndex(hash);
     766 | +        if (!pblockindex) {
    

    ---

    ---

    

    MarcoFalke commented at 1:09 PM on January 4, 2019:

    nit: Can leave this out of the cs_main lock to avoid unnecessary indentation and changes?

    ---

    

    promag commented at 1:17 PM on January 4, 2019:

    Yes indeed better.

    ---

    

    promag commented at 3:01 PM on January 4, 2019:

    Done.

34. rpc: Avoid permanent cs_main lock in getblockheader f12e1d0b51
35. promag force-pushed on Jan 4, 2019
36. 
    MarcoFalke commented at 3:08 PM on January 4, 2019: member

    utACK f12e1d0b5117e3688f52a25ed0170d76ecdbf233

37. ryanofsky approved
38. 
    ryanofsky commented at 4:38 PM on January 4, 2019: contributor

    utACK f12e1d0b5117e3688f52a25ed0170d76ecdbf233. No change since last review other than rebase.

39. MarcoFalke added this to the milestone 0.18.0 on Jan 5, 2019
40. 
    laanwj commented at 2:11 PM on January 7, 2019: member

    So to be clear: this is can be done, because it is safe to access the fields on the CBlockIndex instance without the lock?

41. 
    promag commented at 2:55 PM on January 7, 2019: member

    @laanwj to best of my knowledge yes. Looks like nStatus is the only field that needs the lock, which is not used.

42. 
    jnewbery commented at 9:27 PM on January 7, 2019: member

    utACK f12e1d0b5117e3688f52a25ed0170d76ecdbf233.

    Do you intend to do the same for getblock? Not holding cs_main while reading a block from disk and serializing into json seems like a big win.

43. laanwj merged this on Jan 8, 2019
44. laanwj closed this on Jan 8, 2019

    ---

45. laanwj referenced this in commit 29a9f07743 on Jan 8, 2019
46. promag deleted the branch on Jan 8, 2019
47. 
    promag commented at 3:20 PM on January 8, 2019: member

    @jnewbery #11913 #13903 are related to that.

48. promag cross-referenced this on May 2, 2019 from issue rpc: Serialize in getblock without cs_main by MarcoFalke
49. LarryRuane referenced this in commit 4b3f1d8190 on Apr 29, 2021
50. LarryRuane cross-referenced this on May 4, 2021 from issue Bitcoin 0.18 locking PRs by LarryRuane
51. LarryRuane referenced this in commit 7a4a025b06 on Jun 1, 2021
52. Munkybooty referenced this in commit f3474f2102 on Aug 20, 2021
53. Munkybooty referenced this in commit 512b959152 on Aug 20, 2021
54. Munkybooty referenced this in commit a94fb9395d on Aug 21, 2021
55. Munkybooty referenced this in commit d69fde57c6 on Aug 23, 2021
56. Munkybooty referenced this in commit 2c2eba6629 on Aug 24, 2021
57. Munkybooty referenced this in commit 314e1f4fb3 on Aug 24, 2021
58. UdjinM6 referenced this in commit df08b0a6e6 on Aug 24, 2021
59. Munkybooty referenced this in commit 60d7fd708c on Aug 24, 2021
60. bitcoin locked this on Dec 16, 2021

Contributors

promagryanofskyjnewberyMarcoFalkelaanwj

Milestone
0.18.0