From 1e9925435ec236a6dffe5fe3829d1e502de3c127 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Thu, 21 Sep 2023 00:26:52 +0000 Subject: [PATCH] Reuse fork choice read lock instead of re-acquiring it immediately (#4688) ## Issue Addressed I went through the code base and look for places where we acquire fork choice locks (after the deadlock bug was found and fixed in #4687), and discovered an instance where we re-acquire a lock immediately after dropping it. This shouldn't cause deadlock like the other issue, but is slightly less efficient. --- beacon_node/beacon_chain/src/block_verification.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 3654484e1f..ef7f1b3394 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -754,22 +754,16 @@ impl GossipVerifiedBlock { // reboot if the `observed_block_producers` cache is empty. In that case, without this // check, we will load the parent and state from disk only to find out later that we // already know this block. - if chain - .canonical_head - .fork_choice_read_lock() - .contains_block(&block_root) - { + let fork_choice_read_lock = chain.canonical_head.fork_choice_read_lock(); + if fork_choice_read_lock.contains_block(&block_root) { return Err(BlockError::BlockIsAlreadyKnown); } // Do not process a block that doesn't descend from the finalized root. // // We check this *before* we load the parent so that we can return a more detailed error. - check_block_is_finalized_checkpoint_or_descendant( - chain, - &chain.canonical_head.fork_choice_read_lock(), - &block, - )?; + check_block_is_finalized_checkpoint_or_descendant(chain, &fork_choice_read_lock, &block)?; + drop(fork_choice_read_lock); let block_epoch = block.slot().epoch(T::EthSpec::slots_per_epoch()); let (parent_block, block) = verify_parent_block_is_known(chain, block)?;