From e0de0d8835096d15eef58602240623f33583d9be Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Fri, 15 May 2026 13:42:25 +0100 Subject: [PATCH 01/11] NYM-583: Avoid corrupted database on Windows. On Windows, the database can become corrupted if the client is killed while it is running. This is fixed by ensuring the database file is properly closed. --- .../client/base_client/non_wasm_helpers.rs | 62 ++++++++++++++++--- nym-sqlx-pool-guard/src/lib.rs | 18 ++++++ 2 files changed, 72 insertions(+), 8 deletions(-) diff --git a/common/client-core/src/client/base_client/non_wasm_helpers.rs b/common/client-core/src/client/base_client/non_wasm_helpers.rs index 335ca8c522..cd61760d3c 100644 --- a/common/client-core/src/client/base_client/non_wasm_helpers.rs +++ b/common/client-core/src/client/base_client/non_wasm_helpers.rs @@ -11,11 +11,17 @@ use nym_bandwidth_controller::BandwidthController; use nym_client_core_gateways_storage::OnDiskGatewaysDetails; use nym_credential_storage::storage::Storage as CredentialStorage; use nym_validator_client::{QueryHttpRpcNyxdClient, nyxd}; -use std::{io, path::Path}; +use std::{io, path::Path, time::Duration}; use time::OffsetDateTime; use tracing::{error, info, trace}; use url::Url; +/// Maximum rename retry attempts when the database file is temporarily locked. +const ARCHIVE_MAX_RETRY_ATTEMPTS: u8 = 15; + +/// Delay between archive rename retry attempts. +const ARCHIVE_RETRY_DELAY: Duration = Duration::from_millis(200); + async fn setup_fresh_backend>( db_path: P, surb_config: &config::ReplySurbs, @@ -74,13 +80,53 @@ async fn archive_corrupted_database>(db_path: P) -> io::Result<() }; let renamed = db_path.with_extension(new_extension); - tokio::fs::rename(db_path, &renamed).await.inspect_err(|_| { - error!( - "Failed to rename corrupt database file: {} to {}", - db_path.display(), - renamed.display() - ); - }) + // On Windows, a previously cancelled connection may still hold a file handle open + // (ERROR_SHARING_VIOLATION, os error 32), temporarily preventing the rename. The + // SqlitePoolGuard::drop() spawns an async close task for exactly this situation, so + // we retry with a short delay to give that task time to complete. + let mut last_err = None; + for attempt in 0..ARCHIVE_MAX_RETRY_ATTEMPTS { + match tokio::fs::rename(db_path, &renamed).await { + Ok(()) => return Ok(()), + Err(e) if is_file_locked_error(&e) && (attempt + 1) < ARCHIVE_MAX_RETRY_ATTEMPTS => { + trace!( + "Database file is temporarily locked, retrying archive \ + (attempt {}/{}): {e}", + attempt + 1, + ARCHIVE_MAX_RETRY_ATTEMPTS + ); + tokio::time::sleep(ARCHIVE_RETRY_DELAY).await; + last_err = Some(e); + } + Err(e) => { + last_err = Some(e); + break; + } + } + } + + let err = last_err.unwrap(); + error!( + "Failed to rename corrupt database file: {} to {}", + db_path.display(), + renamed.display() + ); + Err(err) +} + +/// Returns `true` when the IO error indicates a temporary file lock held by another handle +/// within the same process. Only meaningful on Windows; always `false` elsewhere. +fn is_file_locked_error(e: &io::Error) -> bool { + #[cfg(windows)] + { + // ERROR_SHARING_VIOLATION = 32, ERROR_LOCK_VIOLATION = 33 + matches!(e.raw_os_error(), Some(32) | Some(33)) + } + #[cfg(not(windows))] + { + let _ = e; + false + } } pub async fn setup_fs_reply_surb_backend>( diff --git a/nym-sqlx-pool-guard/src/lib.rs b/nym-sqlx-pool-guard/src/lib.rs index 3e16683de3..6eb9b82387 100644 --- a/nym-sqlx-pool-guard/src/lib.rs +++ b/nym-sqlx-pool-guard/src/lib.rs @@ -120,6 +120,24 @@ impl SqlitePoolGuard { } } +impl Drop for SqlitePoolGuard { + fn drop(&mut self) { + if !self.connection_pool.is_closed() { + // When the guard is dropped without an explicit call to close() — for example, + // due to async task cancellation — the underlying pool must still be closed to + // promptly release OS file handles. On Windows, an open file handle prevents + // rename/delete operations on the same file (ERROR_SHARING_VIOLATION, os error + // 32), which would break the next connection attempt's database recovery logic. + let pool = self.connection_pool.clone(); + if let Ok(handle) = tokio::runtime::Handle::try_current() { + handle.spawn(async move { + pool.close().await; + }); + } + } + } +} + impl Deref for SqlitePoolGuard { type Target = sqlx::SqlitePool; From bafed70f51d012140697d397c4f0f732bfbc135d Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Fri, 15 May 2026 13:52:41 +0100 Subject: [PATCH 02/11] Use `unwrap_or_default()`. --- common/client-core/src/client/base_client/non_wasm_helpers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/client-core/src/client/base_client/non_wasm_helpers.rs b/common/client-core/src/client/base_client/non_wasm_helpers.rs index cd61760d3c..b9b08e6001 100644 --- a/common/client-core/src/client/base_client/non_wasm_helpers.rs +++ b/common/client-core/src/client/base_client/non_wasm_helpers.rs @@ -105,7 +105,7 @@ async fn archive_corrupted_database>(db_path: P) -> io::Result<() } } - let err = last_err.unwrap(); + let err = last_err.unwrap_or_default(); error!( "Failed to rename corrupt database file: {} to {}", db_path.display(), From c7e425509501576042ce5874dfd24c6887ae90c8 Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Fri, 15 May 2026 13:58:36 +0100 Subject: [PATCH 03/11] Avoid use of `unwrap()`. --- .../client/base_client/non_wasm_helpers.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/common/client-core/src/client/base_client/non_wasm_helpers.rs b/common/client-core/src/client/base_client/non_wasm_helpers.rs index b9b08e6001..11b711cedd 100644 --- a/common/client-core/src/client/base_client/non_wasm_helpers.rs +++ b/common/client-core/src/client/base_client/non_wasm_helpers.rs @@ -84,7 +84,6 @@ async fn archive_corrupted_database>(db_path: P) -> io::Result<() // (ERROR_SHARING_VIOLATION, os error 32), temporarily preventing the rename. The // SqlitePoolGuard::drop() spawns an async close task for exactly this situation, so // we retry with a short delay to give that task time to complete. - let mut last_err = None; for attempt in 0..ARCHIVE_MAX_RETRY_ATTEMPTS { match tokio::fs::rename(db_path, &renamed).await { Ok(()) => return Ok(()), @@ -96,22 +95,28 @@ async fn archive_corrupted_database>(db_path: P) -> io::Result<() ARCHIVE_MAX_RETRY_ATTEMPTS ); tokio::time::sleep(ARCHIVE_RETRY_DELAY).await; - last_err = Some(e); } Err(e) => { - last_err = Some(e); - break; + error!( + "Failed to rename corrupt database file: {} to {}", + db_path.display(), + renamed.display() + ); + return Err(e); } } } - let err = last_err.unwrap_or_default(); + // Reached only when every attempt was blocked by a file lock. error!( - "Failed to rename corrupt database file: {} to {}", + "Failed to rename corrupt database file after {} attempts: {} to {}", + ARCHIVE_MAX_RETRY_ATTEMPTS, db_path.display(), renamed.display() ); - Err(err) + Err(io::Error::other( + "corrupt database archive blocked by persistent file lock", + )) } /// Returns `true` when the IO error indicates a temporary file lock held by another handle From b45351bb0c47a69cd19741607f629a614a037977 Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Fri, 15 May 2026 14:11:21 +0100 Subject: [PATCH 04/11] Avoid async calls in Drop. --- .../client/base_client/non_wasm_helpers.rs | 8 +++--- nym-sqlx-pool-guard/src/lib.rs | 26 ++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/common/client-core/src/client/base_client/non_wasm_helpers.rs b/common/client-core/src/client/base_client/non_wasm_helpers.rs index 11b711cedd..ab57caf0cf 100644 --- a/common/client-core/src/client/base_client/non_wasm_helpers.rs +++ b/common/client-core/src/client/base_client/non_wasm_helpers.rs @@ -80,10 +80,10 @@ async fn archive_corrupted_database>(db_path: P) -> io::Result<() }; let renamed = db_path.with_extension(new_extension); - // On Windows, a previously cancelled connection may still hold a file handle open - // (ERROR_SHARING_VIOLATION, os error 32), temporarily preventing the rename. The - // SqlitePoolGuard::drop() spawns an async close task for exactly this situation, so - // we retry with a short delay to give that task time to complete. + // On Windows, sqlx may release its OS file handles asynchronously after + // pool.close() returns, briefly keeping the file locked + // (ERROR_SHARING_VIOLATION, os error 32). Retry with a short delay to + // give the OS time to flush the remaining handles. for attempt in 0..ARCHIVE_MAX_RETRY_ATTEMPTS { match tokio::fs::rename(db_path, &renamed).await { Ok(()) => return Ok(()), diff --git a/nym-sqlx-pool-guard/src/lib.rs b/nym-sqlx-pool-guard/src/lib.rs index 6eb9b82387..f58398f942 100644 --- a/nym-sqlx-pool-guard/src/lib.rs +++ b/nym-sqlx-pool-guard/src/lib.rs @@ -56,7 +56,11 @@ impl SqlitePoolGuard { &self.database_path } - /// Close udnerlying sqlite pool and wait for files to be closed before returning. + /// Close the underlying sqlite pool and wait for OS file handles to be released. + /// + /// **Callers must invoke this method before dropping the guard.** The `Drop` impl does + /// not perform async cleanup; it only logs a warning when the pool was not closed + /// beforehand. pub async fn close(&self) { // Avoid waiting for db files once the pool is marked closed to ensure that we don't wait on some other sqlite pool to close the database. if !self.connection_pool.is_closed() { @@ -122,18 +126,16 @@ impl SqlitePoolGuard { impl Drop for SqlitePoolGuard { fn drop(&mut self) { + // Spawning async tasks from Drop is an anti-pattern: tasks may not execute during + // runtime shutdown, and there may be no runtime at all. Callers are responsible + // for calling `close()` before dropping this guard. Log a warning here so that + // missed explicit-close call sites are visible during development. if !self.connection_pool.is_closed() { - // When the guard is dropped without an explicit call to close() — for example, - // due to async task cancellation — the underlying pool must still be closed to - // promptly release OS file handles. On Windows, an open file handle prevents - // rename/delete operations on the same file (ERROR_SHARING_VIOLATION, os error - // 32), which would break the next connection attempt's database recovery logic. - let pool = self.connection_pool.clone(); - if let Ok(handle) = tokio::runtime::Handle::try_current() { - handle.spawn(async move { - pool.close().await; - }); - } + tracing::warn!( + path = %self.database_path.display(), + "SqlitePoolGuard dropped without explicit close(); \ + OS file handles may not be released promptly" + ); } } } From 657aa9f86f98da83cd713ad8455795bc834fc9bf Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Fri, 15 May 2026 14:43:18 +0100 Subject: [PATCH 05/11] Ensure storage is shutdown. --- common/client-core/surb-storage/src/backend/fs_backend/mod.rs | 4 +++- common/client-core/surb-storage/src/lib.rs | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/common/client-core/surb-storage/src/backend/fs_backend/mod.rs b/common/client-core/surb-storage/src/backend/fs_backend/mod.rs index 625913e174..9c0c465433 100644 --- a/common/client-core/surb-storage/src/backend/fs_backend/mod.rs +++ b/common/client-core/surb-storage/src/backend/fs_backend/mod.rs @@ -337,6 +337,8 @@ impl ReplyStorageBackend for Backend { } async fn stop_storage_session(self) -> Result<(), Self::StorageError> { - self.stop_client_use().await + let result = self.stop_client_use().await; + self.shutdown().await; + result } } diff --git a/common/client-core/surb-storage/src/lib.rs b/common/client-core/surb-storage/src/lib.rs index ffd7ad85f6..f88a746ad2 100644 --- a/common/client-core/surb-storage/src/lib.rs +++ b/common/client-core/surb-storage/src/lib.rs @@ -48,6 +48,7 @@ where debug!("Started PersistentReplyStorage"); if let Err(err) = self.backend.start_storage_session().await { error!("failed to start the storage session - {err}"); + self.backend.stop_storage_session().await.ok(); return; } From ad9d66ef3a50a266b6015810d4269adc858e9a79 Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Tue, 19 May 2026 15:39:15 +0100 Subject: [PATCH 06/11] Use an `Arc` in order to detect if this is the last instance before warning. --- nym-sqlx-pool-guard/src/lib.rs | 84 +++++++++++++++++++++++----------- 1 file changed, 57 insertions(+), 27 deletions(-) diff --git a/nym-sqlx-pool-guard/src/lib.rs b/nym-sqlx-pool-guard/src/lib.rs index f58398f942..3e0a64e6ee 100644 --- a/nym-sqlx-pool-guard/src/lib.rs +++ b/nym-sqlx-pool-guard/src/lib.rs @@ -3,8 +3,9 @@ use std::{ io, - ops::{Deref, DerefMut}, + ops::Deref, path::{Path, PathBuf}, + sync::Arc, time::Duration, }; @@ -26,10 +27,8 @@ const CHECK_FILES_CLOSED_MAX_ATTEMPTS: u8 = 20; /// Delay between file checks const CHECK_FILES_CLOSED_RETRY_DELAY: Duration = Duration::from_millis(100); -/// `sqlx::SqlitePool` wrapper providing a workaround for the [known bug](https://github.com/launchbadge/sqlx/issues/3217). -/// In principle after requesting to close the sqlite pool, the wrapper monitors open file descriptor and polls periodically until all database files are closed. -#[derive(Debug, Clone)] -pub struct SqlitePoolGuard { +#[derive(Debug)] +struct SqlitePoolGuardInner { /// Path to sqlite database file. database_path: PathBuf, @@ -37,6 +36,18 @@ pub struct SqlitePoolGuard { connection_pool: sqlx::SqlitePool, } +/// `sqlx::SqlitePool` wrapper providing a workaround for the [known bug](https://github.com/launchbadge/sqlx/issues/3217). +/// In principle after requesting to close the sqlite pool, the wrapper monitors open file descriptor and polls periodically until all database files are closed. +/// +/// This type is cheaply [`Clone`]-able: all clones share the same underlying pool and the same +/// reference count. The `Drop` impl only emits a warning when the **last** reference is dropped +/// without an explicit [`close`](Self::close) call, so it is safe to clone this guard temporarily +/// (e.g. to pass into a spawned task) without triggering spurious warnings. +#[derive(Debug, Clone)] +pub struct SqlitePoolGuard { + inner: Arc, +} + impl SqlitePoolGuard { /// Create new instance providing path to database and connection pool pub fn new(connection_pool: sqlx::SqlitePool) -> Self { @@ -46,14 +57,16 @@ impl SqlitePoolGuard { .to_path_buf(); Self { - database_path, - connection_pool, + inner: Arc::new(SqlitePoolGuardInner { + database_path, + connection_pool, + }), } } /// Returns database path pub fn database_path(&self) -> &Path { - &self.database_path + &self.inner.database_path } /// Close the underlying sqlite pool and wait for OS file handles to be released. @@ -63,14 +76,14 @@ impl SqlitePoolGuard { /// beforehand. pub async fn close(&self) { // Avoid waiting for db files once the pool is marked closed to ensure that we don't wait on some other sqlite pool to close the database. - if !self.connection_pool.is_closed() { - tracing::info!("Closing sqlite pool: {}", self.database_path.display()); + if !self.inner.connection_pool.is_closed() { + tracing::info!("Closing sqlite pool: {}", self.inner.database_path.display()); self.close_pool_inner().await.ok(); } } async fn close_pool_inner(&self) -> std::io::Result<()> { - self.connection_pool.close().await; + self.inner.connection_pool.close().await; self.wait_for_db_files_close().await.inspect_err(|e| { tracing::error!("Failed to wait for file to close: {e}"); @@ -81,15 +94,16 @@ impl SqlitePoolGuard { fn all_database_files(&self) -> Vec { let mut database_files = vec![]; let canonical_path = self + .inner .database_path .canonicalize() .inspect_err(|e| { tracing::error!( "Failed to canonicalize path: {}. Cause: {e}", - self.database_path.display() + self.inner.database_path.display() ); }) - .unwrap_or(self.database_path.clone()); + .unwrap_or(self.inner.database_path.clone()); if let Some(ext) = canonical_path.extension() { for added_ext in ["-shm", "-wal"] { @@ -126,15 +140,9 @@ impl SqlitePoolGuard { impl Drop for SqlitePoolGuard { fn drop(&mut self) { - // Spawning async tasks from Drop is an anti-pattern: tasks may not execute during - // runtime shutdown, and there may be no runtime at all. Callers are responsible - // for calling `close()` before dropping this guard. Log a warning here so that - // missed explicit-close call sites are visible during development. - if !self.connection_pool.is_closed() { + if Arc::strong_count(&self.inner) == 1 && !self.inner.connection_pool.is_closed() { tracing::warn!( - path = %self.database_path.display(), - "SqlitePoolGuard dropped without explicit close(); \ - OS file handles may not be released promptly" + "SqlitePoolGuard dropped without explicit close(); path={}", self.inner.database_path.display() ); } } @@ -144,15 +152,10 @@ impl Deref for SqlitePoolGuard { type Target = sqlx::SqlitePool; fn deref(&self) -> &Self::Target { - &self.connection_pool + &self.inner.connection_pool } } -impl DerefMut for SqlitePoolGuard { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.connection_pool - } -} #[cfg(test)] mod tests { use sqlx::{ @@ -197,4 +200,31 @@ mod tests { assert!(guard.close_pool_inner().await.is_ok()); tokio::fs::remove_file(database_path).await.unwrap(); } + + #[tokio::test] + async fn test_clone_drop_no_warning() { + // Cloning the guard and dropping the clone should not warn because the original is still alive. + let temp_dir = tempfile::tempdir().unwrap(); + let database_path = temp_dir.path().join("storage2.db"); + + let opts = sqlx::sqlite::SqliteConnectOptions::new() + .journal_mode(sqlx::sqlite::SqliteJournalMode::Wal) + .synchronous(SqliteSynchronous::Normal) + .auto_vacuum(SqliteAutoVacuum::Incremental) + .filename(database_path.clone()) + .create_if_missing(true) + .disable_statement_logging(); + let connection_pool = sqlx::SqlitePool::connect_with(opts).await.unwrap(); + let guard = SqlitePoolGuard::new(connection_pool); + + { + let _clone = guard.clone(); + assert_eq!(Arc::strong_count(&guard.inner), 2); + // _clone drops here - should NOT warn since guard still holds a reference + } + assert_eq!(Arc::strong_count(&guard.inner), 1); + + guard.close().await; + tokio::fs::remove_file(database_path).await.unwrap(); + } } From 3c6d88397ece7b966fe1145e91ebe1153687001a Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Tue, 19 May 2026 15:51:34 +0100 Subject: [PATCH 07/11] Formatting. --- nym-sqlx-pool-guard/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/nym-sqlx-pool-guard/src/lib.rs b/nym-sqlx-pool-guard/src/lib.rs index 3e0a64e6ee..a66a0b9167 100644 --- a/nym-sqlx-pool-guard/src/lib.rs +++ b/nym-sqlx-pool-guard/src/lib.rs @@ -77,7 +77,10 @@ impl SqlitePoolGuard { pub async fn close(&self) { // Avoid waiting for db files once the pool is marked closed to ensure that we don't wait on some other sqlite pool to close the database. if !self.inner.connection_pool.is_closed() { - tracing::info!("Closing sqlite pool: {}", self.inner.database_path.display()); + tracing::info!( + "Closing sqlite pool: {}", + self.inner.database_path.display() + ); self.close_pool_inner().await.ok(); } } @@ -142,7 +145,8 @@ impl Drop for SqlitePoolGuard { fn drop(&mut self) { if Arc::strong_count(&self.inner) == 1 && !self.inner.connection_pool.is_closed() { tracing::warn!( - "SqlitePoolGuard dropped without explicit close(); path={}", self.inner.database_path.display() + "SqlitePoolGuard dropped without explicit close(); path={}", + self.inner.database_path.display() ); } } From 4a266a73482c31f3f30e77df71ab37cf1e651118 Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Tue, 19 May 2026 16:43:03 +0100 Subject: [PATCH 08/11] More explicit closing of the database before drop. --- common/bandwidth-controller/src/traits.rs | 10 ++++++++++ common/client-core/src/client/base_client/mod.rs | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/common/bandwidth-controller/src/traits.rs b/common/bandwidth-controller/src/traits.rs index bb958f482b..1ff14027ef 100644 --- a/common/bandwidth-controller/src/traits.rs +++ b/common/bandwidth-controller/src/traits.rs @@ -25,6 +25,8 @@ pub trait BandwidthTicketProvider: Send + Sync { ) -> Result; async fn get_upgrade_mode_token(&self) -> Result, BandwidthControllerError>; + + async fn close(&self) {} } #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] @@ -56,6 +58,10 @@ where .map_err(|_| BandwidthControllerError::MalformedUpgradeModeToken)?; Ok(Some(token)) } + + async fn close(&self) { + self.storage.close().await; + } } #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] @@ -75,4 +81,8 @@ impl BandwidthTicketProvider for Box async fn get_upgrade_mode_token(&self) -> Result, BandwidthControllerError> { (**self).get_upgrade_mode_token().await } + + async fn close(&self) { + (**self).close().await; + } } diff --git a/common/client-core/src/client/base_client/mod.rs b/common/client-core/src/client/base_client/mod.rs index 7b27d7b1b1..253da7d6c6 100644 --- a/common/client-core/src/client/base_client/mod.rs +++ b/common/client-core/src/client/base_client/mod.rs @@ -1023,6 +1023,16 @@ where let encryption_keys = init_res.client_keys.encryption_keypair(); let identity_keys = init_res.client_keys.identity_keypair(); + let credential_store_for_close = credential_store.clone(); + let close_credential_token = shutdown_tracker.clone_shutdown_token(); + shutdown_tracker.try_spawn_named( + async move { + close_credential_token.cancelled().await; + credential_store_for_close.close().await; + }, + "CredentialStorage::close_on_shutdown", + ); + // the components are started in very specific order. Unless you know what you are doing, // do not change that. let bandwidth_controller = self From 8795cbe407fb40c525985d88779bbd4081e6d9ca Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Wed, 20 May 2026 09:28:38 +0100 Subject: [PATCH 09/11] It's not an error if we cannot close the database files. --- nym-sqlx-pool-guard/src/lib.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/nym-sqlx-pool-guard/src/lib.rs b/nym-sqlx-pool-guard/src/lib.rs index a66a0b9167..d1b3808bd9 100644 --- a/nym-sqlx-pool-guard/src/lib.rs +++ b/nym-sqlx-pool-guard/src/lib.rs @@ -88,9 +88,23 @@ impl SqlitePoolGuard { async fn close_pool_inner(&self) -> std::io::Result<()> { self.inner.connection_pool.close().await; - self.wait_for_db_files_close().await.inspect_err(|e| { - tracing::error!("Failed to wait for file to close: {e}"); - }) + if let Err(e) = self.wait_for_db_files_close().await { + if e.kind() == std::io::ErrorKind::TimedOut { + tracing::warn!( + "Timed out waiting for OS file handles for sqlite database to be released; \ + another connection to the same file may still be open. Path = {}", + self.inner.database_path.display() + ); + } else { + tracing::warn!( + "Failed to wait for sqlite database file handles to be released: Path = {}. Error = {}", + self.inner.database_path.display(), + e + ); + } + } + + Ok(()) } /// Returns all database files, including shm and wal files. From bad438b5ad43778a8231c2d52ce09391d8d36fd7 Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Wed, 20 May 2026 12:24:20 +0100 Subject: [PATCH 10/11] Address review comments. --- Cargo.lock | 1 + common/client-core/surb-storage/src/lib.rs | 7 ++++--- nym-sqlx-pool-guard/Cargo.toml | 1 + nym-sqlx-pool-guard/src/lib.rs | 6 +++++- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0eadd13981..dbdc7f892d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8353,6 +8353,7 @@ dependencies = [ "tokio", "tracing", "tracing-subscriber", + "tracing-test", "windows 0.61.3", ] diff --git a/common/client-core/surb-storage/src/lib.rs b/common/client-core/surb-storage/src/lib.rs index f88a746ad2..f79127b422 100644 --- a/common/client-core/surb-storage/src/lib.rs +++ b/common/client-core/surb-storage/src/lib.rs @@ -56,10 +56,11 @@ where info!("PersistentReplyStorage is flushing all reply-related data to underlying storage"); if let Err(err) = self.backend.flush_surb_storage(&mem_state).await { - error!("failed to flush our reply-related data to the persistent storage: {err}") - } else { - info!("Data flush is complete") + error!("failed to flush our reply-related data to the persistent storage: {err}"); + self.backend.stop_storage_session().await.ok(); + return; } + info!("Data flush is complete"); if let Err(err) = self.backend.stop_storage_session().await { error!("failed to properly stop the storage session - {err}. We might not be able to smoothly restore it") diff --git a/nym-sqlx-pool-guard/Cargo.toml b/nym-sqlx-pool-guard/Cargo.toml index 85622863ad..e4b86ad2c5 100644 --- a/nym-sqlx-pool-guard/Cargo.toml +++ b/nym-sqlx-pool-guard/Cargo.toml @@ -42,3 +42,4 @@ windows = { version = "0.61", features = [ [dev-dependencies] tempfile.workspace = true tracing-subscriber.workspace = true +tracing-test.workspace = true diff --git a/nym-sqlx-pool-guard/src/lib.rs b/nym-sqlx-pool-guard/src/lib.rs index d1b3808bd9..8ded2cfb61 100644 --- a/nym-sqlx-pool-guard/src/lib.rs +++ b/nym-sqlx-pool-guard/src/lib.rs @@ -180,6 +180,7 @@ mod tests { ConnectOptions, Executor, sqlite::{SqliteAutoVacuum, SqliteSynchronous}, }; + use tracing_test::traced_test; use super::*; @@ -219,6 +220,7 @@ mod tests { tokio::fs::remove_file(database_path).await.unwrap(); } + #[traced_test] #[tokio::test] async fn test_clone_drop_no_warning() { // Cloning the guard and dropping the clone should not warn because the original is still alive. @@ -238,9 +240,11 @@ mod tests { { let _clone = guard.clone(); assert_eq!(Arc::strong_count(&guard.inner), 2); - // _clone drops here - should NOT warn since guard still holds a reference } assert_eq!(Arc::strong_count(&guard.inner), 1); + assert!(!logs_contain( + "SqlitePoolGuard dropped without explicit close" + )); guard.close().await; tokio::fs::remove_file(database_path).await.unwrap(); From 64c68d7b7686e8da07b10212d5367ea8e6b7da48 Mon Sep 17 00:00:00 2001 From: Andy Duplain Date: Thu, 21 May 2026 09:13:31 +0100 Subject: [PATCH 11/11] Fix tests. --- Cargo.lock | 1 - nym-sqlx-pool-guard/Cargo.toml | 1 - nym-sqlx-pool-guard/src/lib.rs | 5 +---- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dbdc7f892d..8ddcc03d1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8352,7 +8352,6 @@ dependencies = [ "tempfile", "tokio", "tracing", - "tracing-subscriber", "tracing-test", "windows 0.61.3", ] diff --git a/nym-sqlx-pool-guard/Cargo.toml b/nym-sqlx-pool-guard/Cargo.toml index e4b86ad2c5..ed980db3e1 100644 --- a/nym-sqlx-pool-guard/Cargo.toml +++ b/nym-sqlx-pool-guard/Cargo.toml @@ -41,5 +41,4 @@ windows = { version = "0.61", features = [ [dev-dependencies] tempfile.workspace = true -tracing-subscriber.workspace = true tracing-test.workspace = true diff --git a/nym-sqlx-pool-guard/src/lib.rs b/nym-sqlx-pool-guard/src/lib.rs index 8ded2cfb61..8e0d400c65 100644 --- a/nym-sqlx-pool-guard/src/lib.rs +++ b/nym-sqlx-pool-guard/src/lib.rs @@ -184,12 +184,9 @@ mod tests { use super::*; + #[traced_test] #[tokio::test] async fn test_wait_close() { - tracing_subscriber::fmt() - .with_max_level(tracing::Level::TRACE) - .init(); - let temp_dir = tempfile::tempdir().unwrap(); let database_path = temp_dir.path().join("storage.db");