Avoid holding lock guards in match expressions (#24805)

* Avoid holding bank forks read lock for RPC requests

* Avoid using lock guards in temporaries

* revert fetch stage change

core/src/banking_stage.rs

@@ -2094,11 +2094,11 @@ fn next_leader_x<F>(
 where
     F: FnOnce(&ContactInfo) -> SocketAddr,
 {
-    if let Some(leader_pubkey) = poh_recorder
+    let leader_pubkey = poh_recorder
         .lock()
         .unwrap()
-        .leader_after_n_slots(FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET)
-    {
+        .leader_after_n_slots(FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET);
+    if let Some(leader_pubkey) = leader_pubkey {
         cluster_info.lookup_contact_info(&leader_pubkey, port_selector)
     } else {
         None

core/src/cluster_info_vote_listener.rs

@@ -366,7 +366,8 @@ impl ClusterInfoVoteListener {
                 // Always set this to avoid taking the poh lock too often
                 time_since_lock = Instant::now();
                 // We will take this lock at most once every `BANK_SEND_VOTES_LOOP_SLEEP_MS`
-                if let Some(current_working_bank) = poh_recorder.lock().unwrap().bank() {
+                let current_working_bank = poh_recorder.lock().unwrap().bank();
+                if let Some(current_working_bank) = current_working_bank {
                     Self::check_for_leader_bank_and_send_votes(
                         &mut bank_vote_sender_state_option,
                         current_working_bank,

core/src/warm_quic_cache_service.rs

@@ -36,11 +36,11 @@ impl WarmQuicCacheService {
                 let slot_jitter = thread_rng().gen_range(-CACHE_JITTER_SLOT, CACHE_JITTER_SLOT);
                 let mut maybe_last_leader = None;
                 while !exit.load(Ordering::Relaxed) {
-                    if let Some(leader_pubkey) = poh_recorder
+                    let leader_pubkey =  poh_recorder
                         .lock()
                         .unwrap()
-                        .leader_after_n_slots((CACHE_OFFSET_SLOT + slot_jitter) as u64)
-                    {
+                        .leader_after_n_slots((CACHE_OFFSET_SLOT + slot_jitter) as u64);
+                    if let Some(leader_pubkey) = leader_pubkey {
                         if maybe_last_leader
                             .map_or(true, |last_leader| last_leader != leader_pubkey)
                         {

rpc/src/optimistically_confirmed_bank_tracker.rs

@@ -204,7 +204,8 @@ impl OptimisticallyConfirmedBankTracker {
         debug!("received bank notification: {:?}", notification);
         match notification {
             BankNotification::OptimisticallyConfirmed(slot) => {
-                if let Some(bank) = bank_forks.read().unwrap().get(slot) {
+                let bank = bank_forks.read().unwrap().get(slot);
+                if let Some(bank) = bank {
                     let mut w_optimistically_confirmed_bank =
                         optimistically_confirmed_bank.write().unwrap();
 

rpc/src/rpc.rs

@@ -209,10 +209,8 @@ impl JsonRpcRequestProcessor {
     #[allow(deprecated)]
     fn bank(&self, commitment: Option<CommitmentConfig>) -> Arc<Bank> {
         debug!("RPC commitment_config: {:?}", commitment);
-        let r_bank_forks = self.bank_forks.read().unwrap();
 
         let commitment = commitment.unwrap_or_default();
-
         if commitment.is_confirmed() {
             let bank = self
                 .optimistically_confirmed_bank
@@ -250,6 +248,7 @@ impl JsonRpcRequestProcessor {
             CommitmentLevel::SingleGossip | CommitmentLevel::Confirmed => unreachable!(), // SingleGossip variant is deprecated
         };
 
+        let r_bank_forks = self.bank_forks.read().unwrap();
         r_bank_forks.get(slot).unwrap_or_else(|| {
             // We log a warning instead of returning an error, because all known error cases
             // are due to known bugs that should be fixed instead.

rpc/src/rpc_service.rs

@@ -296,8 +296,7 @@ impl RequestMiddleware for RpcRequestMiddleware {
 fn process_rest(bank_forks: &Arc<RwLock<BankForks>>, path: &str) -> Option<String> {
     match path {
         "/v0/circulating-supply" => {
-            let r_bank_forks = bank_forks.read().unwrap();
-            let bank = r_bank_forks.root_bank();
+            let bank = bank_forks.read().unwrap().root_bank();
             let total_supply = bank.capitalization();
             let non_circulating_supply =
                 solana_runtime::non_circulating_supply::calculate_non_circulating_supply(&bank)
@@ -309,8 +308,7 @@ fn process_rest(bank_forks: &Arc<RwLock<BankForks>>, path: &str) -> Option<Strin
             ))
         }
         "/v0/total-supply" => {
-            let r_bank_forks = bank_forks.read().unwrap();
-            let bank = r_bank_forks.root_bank();
+            let bank = bank_forks.read().unwrap().root_bank();
             let total_supply = bank.capitalization();
             Some(format!("{}", lamports_to_sol(total_supply)))
         }

rpc/src/rpc_subscriptions.rs

@@ -145,7 +145,8 @@ where
     X: Clone + Default,
 {
     let mut notified = false;
-    if let Some(bank) = bank_forks.read().unwrap().get(slot) {
+    let bank = bank_forks.read().unwrap().get(slot);
+    if let Some(bank) = bank {
         let results = bank_method(&bank, params);
         let mut w_last_notified_slot = subscription.last_notified_slot.write().unwrap();
         let (filter_results, result_slot) =
@@ -442,7 +443,7 @@ fn initial_last_notified_slot(
     bank_forks: &RwLock<BankForks>,
     block_commitment_cache: &RwLock<BlockCommitmentCache>,
     optimistically_confirmed_bank: &RwLock<OptimisticallyConfirmedBank>,
-) -> Slot {
+) -> Option<Slot> {
     match params {
         SubscriptionParams::Account(params) => {
             let slot = if params.commitment.is_finalized() {
@@ -456,18 +457,10 @@ fn initial_last_notified_slot(
                 block_commitment_cache.read().unwrap().slot()
             };
 
-            if let Some((_account, slot)) = bank_forks
-                .read()
-                .unwrap()
-                .get(slot)
-                .and_then(|bank| bank.get_account_modified_slot(&params.pubkey))
-            {
-                slot
-            } else {
-                0
-            }
+            let bank = bank_forks.read().unwrap().get(slot)?;
+            Some(bank.get_account_modified_slot(&params.pubkey)?.1)
         }
-        _ => 0,
+        _ => None,
     }
 }
 
@@ -754,6 +747,7 @@ impl RpcSubscriptions {
                                     &block_commitment_cache,
                                     &optimistically_confirmed_bank,
                                 )
+                                .unwrap_or(0)
                             });
                         }
                         NotificationEntry::Unsubscribed(params, id) => {
@@ -942,7 +936,8 @@ impl RpcSubscriptions {
                 SubscriptionParams::Block(params) => {
                     num_blocks_found.fetch_add(1, Ordering::Relaxed);
                     if let Some(slot) = slot {
-                        if let Some(bank) = bank_forks.read().unwrap().get(slot) {
+                        let bank = bank_forks.read().unwrap().get(slot);
+                        if let Some(bank) = bank {
                             // We're calling it unnotified in this context
                             // because, logically, it gets set to `last_notified_slot + 1`
                             // on the final iteration of the loop down below.