Significantly reduce GetTransaction cs_main locking (TheBlueMatt) #13903
Conversation
|
What happens if pruning removes the block while we're reading it? |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
@gmaxwell wouldn't the removal fail since the file is opened? |
|
@promag At least on Linux ext4, opened files can be removed cleanly. It would just no longer be possible to reopen them from filename. |
|
So in that case pruning would succeed and in that case LGTM. If I'll try to reproduce. |
|
It seems there is a race condition if the block is pruned after Lines 1094 to 1099 in 8f46454 On unix it should be sufficient to just hold onto |
| if (!GetTransaction(hash, tx, Params().GetConsensus(), hash_block, true, blockindex)) { | ||
| std::string errmsg; | ||
| if (blockindex) { | ||
| LOCK(cs_main); |
There was a problem hiding this comment.
Note: it looks like adding this lock here is just a bugfix, and not directly related to the other changes.
| Needs rebase |
Reading a block from disk does not require
cs_main, so it can be removed or held shorter in functions that read from disk.This is the first commit stolen from #11913 without rebase or otherwise touching it.