Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid permanent cs_main lock in getblockheader #12153

Merged
merged 1 commit into from Jan 8, 2019
Merged

Avoid permanent cs_main lock in getblockheader #12153

merged 1 commit into from Jan 8, 2019

Conversation

promag
Copy link
Member

@promag promag commented Jan 11, 2018 •
edited

This PR reduces the cs_main lock scope in getblockheader RPC.

@promag promag force-pushed the 2018-01-getblockheader branch 4 times, most recently from 7bb7175 to 5e924f4 Compare January 15, 2018 15:10
ryanofsky
ryanofsky reviewed Feb 5, 2018
View reviewed changes
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@jnewbery
Copy link
Contributor

jnewbery commented Apr 3, 2018

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

@promag promag force-pushed the 2018-01-getblockheader branch 2 times, most recently from e167a84 to ba2dad1 Compare June 10, 2018 20:35
@DrahtBot DrahtBot mentioned this pull request Jun 14, 2018
Merged
@DrahtBot DrahtBot mentioned this pull request Jul 6, 2018
Closed
@DrahtBot DrahtBot closed this Aug 2, 2018
@DrahtBot DrahtBot reopened this Aug 2, 2018
@DrahtBot DrahtBot mentioned this pull request Aug 10, 2018
Closed
@DrahtBot DrahtBot mentioned this pull request Sep 10, 2018
Closed
@DrahtBot DrahtBot mentioned this pull request Sep 21, 2018
Merged
@DrahtBot DrahtBot closed this Jan 3, 2019
@fanquake fanquake reopened this Jan 4, 2019
@promag
Copy link
Member Author

promag commented Jan 4, 2019

Rebased.

@bitcoin bitcoin deleted a comment from DrahtBot Jan 4, 2019
@bitcoin bitcoin deleted a comment from DrahtBot Jan 4, 2019
@bitcoin bitcoin deleted a comment from DrahtBot Jan 4, 2019
@bitcoin bitcoin deleted a comment from DrahtBot Jan 4, 2019
maflcko
maflcko reviewed Jan 4, 2019
View reviewed changes
src/rpc/blockchain.cpp Outdated Show resolved Hide resolved
@maflcko
Copy link
Member

maflcko commented Jan 4, 2019

utACK f12e1d0

ryanofsky
ryanofsky approved these changes Jan 4, 2019
View reviewed changes
Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@maflcko maflcko added this to the 0.18.0 milestone Jan 5, 2019
@laanwj
Copy link
Member

laanwj commented Jan 7, 2019

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

@promag
Copy link
Member Author

promag commented Jan 7, 2019

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

@jnewbery
Copy link
Contributor

jnewbery commented Jan 7, 2019

utACK f12e1d0.

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.

@laanwj laanwj merged commit f12e1d0 into bitcoin:master Jan 8, 2019
laanwj added a commit that referenced this pull request Jan 8, 2019
@laanwj
Merge #12153: Avoid permanent cs_main lock in getblockheader
29a9f07 
f12e1d0 rpc: Avoid permanent cs_main lock in getblockheader (João Barbosa)

Pull request description:

  This PR reduces the `cs_main` lock scope in `getblockheader` RPC.

Tree-SHA512: bc51f80e15d1b32d3c7886836457f9929706b6aad9841dafce31ffca444281471b21b56192bb50de774184b9377412f815ad8d3d2439049a7e64d2e59c415767
@promag promag deleted the 2018-01-getblockheader branch January 8, 2019 12:59
@promag
Copy link
Member Author

promag commented Jan 8, 2019

@jnewbery #11913 #13903 are related to that.

@promag promag mentioned this pull request May 2, 2019
Merged
LarryRuane pushed a commit to LarryRuane/zcash that referenced this pull request Apr 29, 2021
@promag @LarryRuane
rpc: Avoid permanent cs_main lock in getblockheader
4b3f1d8 
zcash: cherry picked from commit f12e1d0
zcash: bitcoin/bitcoin#12153
@LarryRuane LarryRuane mentioned this pull request May 4, 2021
Open
LarryRuane pushed a commit to LarryRuane/zcash that referenced this pull request Jun 1, 2021
@promag @LarryRuane
rpc: Avoid permanent cs_main lock in getblockheader
7a4a025 
zcash: cherry picked from commit f12e1d0
zcash: bitcoin/bitcoin#12153
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 20, 2021
@laanwj @Munkybooty
Merge bitcoin#12153: Avoid permanent cs_main lock in getblockheader
f3474f2 
f12e1d0 rpc: Avoid permanent cs_main lock in getblockheader (João Barbosa)

Pull request description:

  This PR reduces the `cs_main` lock scope in `getblockheader` RPC.

Tree-SHA512: bc51f80e15d1b32d3c7886836457f9929706b6aad9841dafce31ffca444281471b21b56192bb50de774184b9377412f815ad8d3d2439049a7e64d2e59c415767
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 20, 2021
@laanwj @Munkybooty
Merge bitcoin#12153: Avoid permanent cs_main lock in getblockheader
512b959 
f12e1d0 rpc: Avoid permanent cs_main lock in getblockheader (João Barbosa)

Pull request description:

  This PR reduces the `cs_main` lock scope in `getblockheader` RPC.

Tree-SHA512: bc51f80e15d1b32d3c7886836457f9929706b6aad9841dafce31ffca444281471b21b56192bb50de774184b9377412f815ad8d3d2439049a7e64d2e59c415767
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 21, 2021
@laanwj @Munkybooty
Merge bitcoin#12153: Avoid permanent cs_main lock in getblockheader
a94fb93 
f12e1d0 rpc: Avoid permanent cs_main lock in getblockheader (João Barbosa)

Pull request description:

  This PR reduces the `cs_main` lock scope in `getblockheader` RPC.

Tree-SHA512: bc51f80e15d1b32d3c7886836457f9929706b6aad9841dafce31ffca444281471b21b56192bb50de774184b9377412f815ad8d3d2439049a7e64d2e59c415767
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 23, 2021
@laanwj @Munkybooty
Merge bitcoin#12153: Avoid permanent cs_main lock in getblockheader
d69fde5 
f12e1d0 rpc: Avoid permanent cs_main lock in getblockheader (João Barbosa)

Pull request description:

  This PR reduces the `cs_main` lock scope in `getblockheader` RPC.

Tree-SHA512: bc51f80e15d1b32d3c7886836457f9929706b6aad9841dafce31ffca444281471b21b56192bb50de774184b9377412f815ad8d3d2439049a7e64d2e59c415767
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 24, 2021
@laanwj @Munkybooty
Merge bitcoin#12153: Avoid permanent cs_main lock in getblockheader
2c2eba6 
f12e1d0 rpc: Avoid permanent cs_main lock in getblockheader (João Barbosa)

Pull request description:

  This PR reduces the `cs_main` lock scope in `getblockheader` RPC.

Tree-SHA512: bc51f80e15d1b32d3c7886836457f9929706b6aad9841dafce31ffca444281471b21b56192bb50de774184b9377412f815ad8d3d2439049a7e64d2e59c415767
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 24, 2021
@laanwj @Munkybooty
Merge bitcoin#12153: Avoid permanent cs_main lock in getblockheader
314e1f4 
f12e1d0 rpc: Avoid permanent cs_main lock in getblockheader (João Barbosa)

Pull request description:

  This PR reduces the `cs_main` lock scope in `getblockheader` RPC.

Tree-SHA512: bc51f80e15d1b32d3c7886836457f9929706b6aad9841dafce31ffca444281471b21b56192bb50de774184b9377412f815ad8d3d2439049a7e64d2e59c415767
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Aug 24, 2021
@laanwj @UdjinM6
Merge bitcoin#12153: Avoid permanent cs_main lock in getblockheader
df08b0a 
f12e1d0 rpc: Avoid permanent cs_main lock in getblockheader (João Barbosa)

Pull request description:

  This PR reduces the `cs_main` lock scope in `getblockheader` RPC.

Tree-SHA512: bc51f80e15d1b32d3c7886836457f9929706b6aad9841dafce31ffca444281471b21b56192bb50de774184b9377412f815ad8d3d2439049a7e64d2e59c415767
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 24, 2021
@laanwj @Munkybooty
Merge bitcoin#12153: Avoid permanent cs_main lock in getblockheader
60d7fd7 
f12e1d0 rpc: Avoid permanent cs_main lock in getblockheader (João Barbosa)

Pull request description:

  This PR reduces the `cs_main` lock scope in `getblockheader` RPC.

Tree-SHA512: bc51f80e15d1b32d3c7886836457f9929706b6aad9841dafce31ffca444281471b21b56192bb50de774184b9377412f815ad8d3d2439049a7e64d2e59c415767
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@maflcko maflcko maflcko left review comments

@ryanofsky ryanofsky ryanofsky approved these changes

Assignees
No one assigned
Labels
RPC/REST/ZMQ
Projects
None yet
Milestone
0.18.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@promag @jnewbery @maflcko @laanwj @ryanofsky @fanquake @DrahtBot