Skip to content

Commit

Permalink
Move EL update spawning
Browse files Browse the repository at this point in the history
  • Loading branch information
paulhauner committed Jun 28, 2022
1 parent 93864b8 commit ab4e1e8
Showing 1 changed file with 13 additions and 22 deletions.
35 changes: 13 additions & 22 deletions beacon_node/beacon_chain/src/recompute_head.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,25 +241,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
// Alias for readability.
let new_head = &canonical_head_read_lock.head_snapshot;

// Update the execution layer since either the head or some finality parameters have
// changed.
//
// Since we're spawning these tasks here *without* awaiting them, they'll happen
// concurrently with the other calls in this function (e.g., cleaning caches and migrating
// things from the hot database into the cold database).
//
// Notably, we're spawning these tasks whilst holding a read-lock on `self.canonical_head`.
// There is a scenario where these spawned tasks will try to take a write-lock on
// `self.canonical_head` (e.g., when the execution status of a block changes). Whilst some
// lock contention might arise, this won't deadlock as long as we drop the read-lock on
// `self.canonical_head` before waiting for these tasks to complete.
let forkchoice_update_parameters = canonical_head_read_lock
.fork_choice
.get_forkchoice_update_parameters()
.ok_or(Error::ForkchoiceUpdateParamsMissing)?;
let el_update_handle =
spawn_execution_layer_updates(self.clone(), forkchoice_update_parameters)?;

// If the head changed, perform some updates.
if let Some((old_head, new_head_proto_block)) = &head_update_params {
if let Err(e) = self.after_new_head(old_head, new_head, new_head_proto_block) {
Expand All @@ -282,11 +263,21 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
}
}

// The read-lock on the canonical head *MUST* be dropped before awaiting the completion of
// the execution layer updates. Those updates might try to take a write-lock on the head, so
// holding the read-lock until they complete may result in a **deadlock**.
// Get the parameters toupdate the execution layer since either the head or some finality
// parameters have changed.
let forkchoice_update_parameters = canonical_head_read_lock
.fork_choice
.get_forkchoice_update_parameters()
.ok_or(Error::ForkchoiceUpdateParamsMissing)?;

// The read-lock on the canonical head *MUST* be dropped before spawning the execution
// layer update tasks since they might try to take a write-lock on the canonical head.
drop(canonical_head_read_lock);

// The read-lock on the canonical head *MUST* be dropped before this call since it might try to take a write-lock on the canonical head.
let el_update_handle =
spawn_execution_layer_updates(self.clone(), forkchoice_update_parameters)?;

Ok(Some(el_update_handle))
}

Expand Down

0 comments on commit ab4e1e8

Please sign in to comment.