From 34f4bfb82a0b13a2c3b65372a141b110f0d63f22 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 29 Jul 2025 14:03:12 +0200 Subject: [PATCH] fix: settings are read from db 20 times a second --- src/backend_task/system_task/mod.rs | 2 + src/context.rs | 72 ++++++++++++++++++++++++- src/database/settings.rs | 15 +++++- src/ui/components/top_panel.rs | 5 +- src/ui/network_chooser_screen.rs | 1 - src/ui/wallets/add_new_wallet_screen.rs | 1 - src/ui/wallets/import_wallet_screen.rs | 1 - 7 files changed, 89 insertions(+), 8 deletions(-) diff --git a/src/backend_task/system_task/mod.rs b/src/backend_task/system_task/mod.rs index cba2e729d..d7a6383d2 100644 --- a/src/backend_task/system_task/mod.rs +++ b/src/backend_task/system_task/mod.rs @@ -48,6 +48,8 @@ impl AppContext { self: &Arc, theme_mode: ThemeMode, ) -> Result { + let _guard = self.invalidate_settings_cache(); + self.db .update_theme_preference(theme_mode) .map_err(|e| e.to_string())?; diff --git a/src/context.rs b/src/context.rs index caaa96b85..c71f28694 100644 --- a/src/context.rs +++ b/src/context.rs @@ -39,10 +39,16 @@ use egui::Context; use rusqlite::Result; use std::collections::{BTreeMap, HashMap}; use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::{Arc, Mutex, RwLock}; +use std::sync::{Arc, Mutex, RwLock, RwLockWriteGuard}; const ANIMATION_REFRESH_TIME: std::time::Duration = std::time::Duration::from_millis(100); +/// A guard that ensures settings cache invalidation happens atomically +/// +/// This guard holds a write lock on the cached settings, preventing reads +/// until the database update is complete and the cache is properly invalidated. +type SettingsCacheGuard<'a> = RwLockWriteGuard<'a, Option>; + #[derive(Debug)] pub struct AppContext { pub(crate) network: Network, @@ -70,6 +76,9 @@ pub struct AppContext { /// This is used to control animations in the UI, such as loading spinners or transitions. /// Disable for automated tests. animate: AtomicBool, + /// Cached settings to avoid expensive database reads + /// Use RwLock to allow multiple readers but exclusive writers for cache invalidation + cached_settings: RwLock>, // subtasks started by the app context, used for graceful shutdown pub(crate) subtasks: Arc, } @@ -176,6 +185,7 @@ impl AppContext { transactions_waiting_for_finality: Mutex::new(BTreeMap::new()), zmq_connection_status: Mutex::new(ZMQConnectionEvent::Disconnected), animate, + cached_settings: RwLock::new(None), subtasks, }; @@ -472,13 +482,73 @@ impl AppContext { /// Updates the `start_root_screen` in the settings table pub fn update_settings(&self, root_screen_type: RootScreenType) -> Result<()> { + let _guard = self.invalidate_settings_cache(); + self.db .insert_or_update_settings(self.network, root_screen_type) } + /// Updates the main password settings + pub fn update_main_password( + &self, + salt: &[u8], + nonce: &[u8], + password_check: &[u8], + ) -> Result<()> { + let _guard = self.invalidate_settings_cache(); + + self.db.update_main_password(salt, nonce, password_check) + } + + /// Updates the Dash Core execution settings + pub fn update_dash_core_execution_settings( + &self, + custom_dash_qt_path: Option, + overwrite_dash_conf: bool, + ) -> Result<()> { + let _guard = self.invalidate_settings_cache(); + + self.db + .update_dash_core_execution_settings(custom_dash_qt_path, overwrite_dash_conf) + } + + /// Invalidates the settings cache and returns a guard + /// + /// The cache is invalidated immediately and the guard prevents concurrent access + /// until the database operation is complete. This ensures atomicity and prevents + /// race conditions regardless of whether the database operation succeeds or fails. + pub fn invalidate_settings_cache(&self) -> SettingsCacheGuard { + let mut guard = self.cached_settings.write().unwrap(); + *guard = None; + guard + } + /// Retrieves the current settings + /// + /// ## Cached + /// + /// This function uses a cache to avoid expensive database operations. + /// The cache is invalidated when settings are updated. + /// + /// Use [`AppContext::invalidate_settings_cache`] to invalidate the cache. pub fn get_settings(&self) -> Result> { + // First, try to read from cache + { + let cache = self.cached_settings.read().unwrap(); + if let Some(ref settings) = *cache { + return Ok(Some(settings.clone())); + } + } + + // Cache miss, read from database let settings = self.db.get_settings()?.map(Settings::from); + + // Update cache with the fresh data + { + let mut cache = self.cached_settings.write().unwrap(); + *cache = settings.clone(); + } + Ok(settings) } diff --git a/src/database/settings.rs b/src/database/settings.rs index 080712ccc..eaa47bad2 100644 --- a/src/database/settings.rs +++ b/src/database/settings.rs @@ -9,6 +9,8 @@ use std::{path::PathBuf, str::FromStr}; impl Database { /// Inserts or updates the settings in the database. This method ensures that only one row exists. + /// + /// Don't call this method directly, use `AppContext` methods instead to ensure proper caching behavior. pub fn insert_or_update_settings( &self, network: Network, @@ -27,6 +29,9 @@ impl Database { Ok(()) } + /// Updates the main password information in the settings table. + /// + /// Don't call this method directly, use `AppContext` methods instead to ensure proper caching behavior. pub fn update_main_password( &self, salt: &[u8], @@ -45,7 +50,9 @@ impl Database { Ok(()) } - + /// Updates the Dash Core execution settings in the settings table. + /// + /// Don't call this method directly, use `AppContext` methods instead to ensure proper caching behavior. pub fn update_dash_core_execution_settings( &self, custom_dash_qt_path: Option, @@ -112,7 +119,9 @@ impl Database { Ok(()) } - + /// Updates the theme preference in the settings table. + /// + /// Don't call this method directly, use `AppContext` methods instead to ensure proper caching behavior. pub fn update_theme_preference(&self, theme_preference: ThemeMode) -> Result<()> { let theme_str = match theme_preference { ThemeMode::Light => "Light", @@ -144,6 +153,8 @@ impl Database { } /// Retrieves the settings from the database. + /// + /// Don't call this method directly, use `AppContext` methods instead to ensure proper caching behavior. #[allow(clippy::type_complexity)] pub fn get_settings( &self, diff --git a/src/ui/components/top_panel.rs b/src/ui/components/top_panel.rs index 6a0d98f65..84e606194 100644 --- a/src/ui/components/top_panel.rs +++ b/src/ui/components/top_panel.rs @@ -159,9 +159,10 @@ fn add_connection_indicator(ui: &mut Ui, app_context: &Arc) -> AppAc let resp = resp.on_hover_text(tip); if resp.clicked() && !connected { - let settings = app_context.db.get_settings().ok().flatten(); + let settings = app_context.get_settings().ok().flatten(); + let (custom_path, overwrite) = settings - .map(|(_, _, _, custom_path, overwrite, _)| (custom_path, overwrite)) + .map(|s| (s.dash_qt_path, s.overwrite_dash_conf)) .unwrap_or((None, true)); if let Some(dash_qt_path) = custom_path { action |= AppAction::BackendTask(BackendTask::CoreTask( diff --git a/src/ui/network_chooser_screen.rs b/src/ui/network_chooser_screen.rs index 4941eeef7..d98ea17c7 100644 --- a/src/ui/network_chooser_screen.rs +++ b/src/ui/network_chooser_screen.rs @@ -120,7 +120,6 @@ impl NetworkChooserScreen { /// TODO: doesn't save local network settings like password yet. fn save(&self) -> Result<(), String> { self.current_app_context() - .db .update_dash_core_execution_settings( self.custom_dash_qt_path.clone(), self.overwrite_dash_conf, diff --git a/src/ui/wallets/add_new_wallet_screen.rs b/src/ui/wallets/add_new_wallet_screen.rs index f993b5bc7..ea5b2d170 100644 --- a/src/ui/wallets/add_new_wallet_screen.rs +++ b/src/ui/wallets/add_new_wallet_screen.rs @@ -99,7 +99,6 @@ impl AddNewWalletScreen { let (encrypted_message, salt, nonce) = encrypt_message(DASH_SECRET_MESSAGE, self.password.as_str())?; self.app_context - .db .update_main_password(&salt, &nonce, &encrypted_message) .map_err(|e| e.to_string())?; } diff --git a/src/ui/wallets/import_wallet_screen.rs b/src/ui/wallets/import_wallet_screen.rs index d09f7aa4f..591b6801a 100644 --- a/src/ui/wallets/import_wallet_screen.rs +++ b/src/ui/wallets/import_wallet_screen.rs @@ -63,7 +63,6 @@ impl ImportWalletScreen { let (encrypted_message, salt, nonce) = encrypt_message(DASH_SECRET_MESSAGE, self.password.as_str())?; self.app_context - .db .update_main_password(&salt, &nonce, &encrypted_message) .map_err(|e| e.to_string())?; }