From c569a3771369e3a53dfc88c4f7ee1ad7c8dcdd55 Mon Sep 17 00:00:00 2001 From: Brooks Date: Tue, 30 May 2023 17:42:33 -0400 Subject: [PATCH] Handle errors when sending an accounts package during shutdown (#31874) --- core/tests/snapshots.rs | 8 +++++++- runtime/src/accounts_background_service.rs | 18 +++++++++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index c72d84b35..a4fc9776f 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -233,7 +233,12 @@ fn run_bank_forks_snapshot_n( if slot % set_root_interval == 0 || slot == last_slot { // set_root should send a snapshot request bank_forks.set_root(bank.slot(), &request_sender, None); - snapshot_request_handler.handle_snapshot_requests(false, 0, &mut None); + snapshot_request_handler.handle_snapshot_requests( + false, + 0, + &mut None, + &AtomicBool::new(false), + ); } } @@ -753,6 +758,7 @@ fn test_bank_forks_incremental_snapshot( false, 0, &mut last_full_snapshot_slot, + &AtomicBool::new(false), ); } diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index f650f84ea..4068e2ff3 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -149,6 +149,7 @@ impl SnapshotRequestHandler { test_hash_calculation: bool, non_snapshot_time_us: u128, last_full_snapshot_slot: &mut Option, + exit: &AtomicBool, ) -> Option> { let ( snapshot_request, @@ -174,6 +175,7 @@ impl SnapshotRequestHandler { last_full_snapshot_slot, snapshot_request, accounts_package_type, + exit, )) } @@ -295,6 +297,7 @@ impl SnapshotRequestHandler { last_full_snapshot_slot: &mut Option, snapshot_request: SnapshotRequest, accounts_package_type: AccountsPackageType, + exit: &AtomicBool, ) -> Result { debug!( "handling snapshot request: {:?}, {:?}", @@ -414,9 +417,15 @@ impl SnapshotRequestHandler { ) } }; - self.accounts_package_sender - .send(accounts_package) - .expect("send accounts package"); + let send_result = self.accounts_package_sender.send(accounts_package); + if let Err(err) = send_result { + // Sending the accounts package should never fail *unless* we're shutting down. + let accounts_package = &err.0; + assert!( + exit.load(Ordering::Relaxed), + "Failed to send accounts package: {err}, {accounts_package:?}" + ); + } snapshot_time.stop(); info!( "Took bank snapshot. accounts package type: {:?}, slot: {}, bank hash: {}", @@ -528,11 +537,13 @@ impl AbsRequestHandlers { test_hash_calculation: bool, non_snapshot_time_us: u128, last_full_snapshot_slot: &mut Option, + exit: &AtomicBool, ) -> Option> { self.snapshot_request_handler.handle_snapshot_requests( test_hash_calculation, non_snapshot_time_us, last_full_snapshot_slot, + exit, ) } } @@ -615,6 +626,7 @@ impl AccountsBackgroundService { test_hash_calculation, non_snapshot_time, &mut last_full_snapshot_slot, + &exit, ) }) .flatten();