From 5596273e6a870029203c4bc17013085104d76bea Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Tue, 3 Nov 2026 08:10:30 -0800 Subject: [PATCH] fix(sqlite): retain verified reader pins through connection close Keep verified foreign SQLite readers together with their inode pins when moving into the shared host reader. Drop the SQLite connection before its raw pin, and verify that cloned host handles retain both descriptors until the final handle is released on Linux. Refs #4513; complements #4720 without addressing the writer pin lifecycle. --- .../src/connection/mod.rs | 11 ++- .../src/runtime/host_sqlite.rs | 89 ++++++++++++++++++- .../runtime/hosts/cursor_composer/sqlite.rs | 6 +- 3 files changed, 97 insertions(+), 9 deletions(-) diff --git a/crates/tracedecay-rusqlite-runtime/src/connection/mod.rs b/crates/tracedecay-rusqlite-runtime/src/connection/mod.rs index 0f4af77d0b..9437ece2bc 100644 --- a/crates/tracedecay-rusqlite-runtime/src/connection/mod.rs +++ b/crates/tracedecay-rusqlite-runtime/src/connection/mod.rs @@ -569,10 +569,13 @@ impl std::error::Error for VerifiedReaderError { /// Foreign-database reader bound to the physical file actually /// opened by SQLite rather than a later pathname observation. pub struct VerifiedReader { - _opened_file: OpenedDatabaseFile, + // Keep the connection before the raw pin in declaration order: closing + // any raw descriptor on Unix can release another SQLite connection's + // process-wide record locks. SQLite must close its handle first. connection: Connection, canonical_path: PathBuf, file_identity: u64, + _opened_file: OpenedDatabaseFile, } pub fn default_vfs_max_pathname() -> Option { @@ -592,8 +595,10 @@ impl VerifiedReader { self.file_identity } - pub fn into_parts(self) -> (Connection, PathBuf, u64) { - (self.connection, self.canonical_path, self.file_identity) + /// The verified canonical pathname. It remains bound to this reader's + /// retained physical file until the SQLite connection is closed. + pub fn canonical_path(&self) -> &Path { + &self.canonical_path } } diff --git a/crates/tracedecay-sessions/src/runtime/host_sqlite.rs b/crates/tracedecay-sessions/src/runtime/host_sqlite.rs index 2775366cc0..b48ccd3cf0 100644 --- a/crates/tracedecay-sessions/src/runtime/host_sqlite.rs +++ b/crates/tracedecay-sessions/src/runtime/host_sqlite.rs @@ -1,14 +1,37 @@ use std::sync::{Arc, Mutex, PoisonError}; +/// A plain SQLite read or a verified foreign read. The verified case retains +/// its inode pin for the whole connection lifetime instead of unpacking and +/// dropping it immediately after admission. +enum SqliteReadHandle { + Plain(rusqlite::Connection), + Verified(tracedecay_rusqlite_runtime::VerifiedReader), +} + +impl SqliteReadHandle { + fn connection(&self) -> &rusqlite::Connection { + match self { + Self::Plain(connection) => connection, + Self::Verified(reader) => reader.connection(), + } + } +} + #[derive(Clone)] pub(crate) struct SqliteReadConn { - inner: Arc>, + inner: Arc>, } impl SqliteReadConn { pub(crate) fn new(connection: rusqlite::Connection) -> Self { Self { - inner: Arc::new(Mutex::new(connection)), + inner: Arc::new(Mutex::new(SqliteReadHandle::Plain(connection))), + } + } + + pub(crate) fn new_verified(reader: tracedecay_rusqlite_runtime::VerifiedReader) -> Self { + Self { + inner: Arc::new(Mutex::new(SqliteReadHandle::Verified(reader))), } } @@ -21,9 +44,69 @@ impl SqliteReadConn { let inner = Arc::clone(&self.inner); tracedecay_runtime_core::native_runtime::spawn_blocking(move || { let guard = inner.lock().unwrap_or_else(PoisonError::into_inner); - body(&guard) + body(guard.connection()) }) .await .ok() } } + +#[cfg(all(test, target_os = "linux"))] +mod tests { + use std::os::unix::fs::MetadataExt; + use std::path::Path; + + use super::SqliteReadConn; + + fn descriptors_for(path: &Path) -> usize { + let identity = std::fs::metadata(path).expect("database metadata"); + std::fs::read_dir("/proc/self/fd") + .expect("process file descriptors") + .filter_map(Result::ok) + .filter(|entry| { + std::fs::metadata(entry.path()).is_ok_and(|metadata| { + metadata.dev() == identity.dev() && metadata.ino() == identity.ino() + }) + }) + .count() + } + + #[test] + fn verified_foreign_reader_retains_its_pin_until_the_last_handle_closes() { + let dir = tempfile::tempdir().expect("test directory"); + let path = dir.path().join("foreign.sqlite3"); + { + let conn = rusqlite::Connection::open(&path).expect("create database"); + conn.execute_batch( + "CREATE TABLE sample(value INTEGER); INSERT INTO sample VALUES (42)", + ) + .expect("seed database"); + } + assert_eq!(descriptors_for(&path), 0); + + let verified = tracedecay_rusqlite_runtime::open_verified_immutable_reader(&path) + .expect("open verified foreign database"); + let pinned_descriptors = descriptors_for(&path); + assert!( + pinned_descriptors >= 2, + "SQLite and its verified identity pin each retain an inode handle" + ); + + let reader = SqliteReadConn::new_verified(verified); + assert_eq!(descriptors_for(&path), pinned_descriptors); + let other = reader.clone(); + drop(reader); + assert_eq!(descriptors_for(&path), pinned_descriptors); + let value: i64 = other + .inner + .lock() + .expect("reader lock") + .connection() + .query_row("SELECT value FROM sample", [], |row| row.get(0)) + .expect("read verified database"); + assert_eq!(value, 42); + + drop(other); + assert_eq!(descriptors_for(&path), 0); + } +} diff --git a/crates/tracedecay-sessions/src/runtime/hosts/cursor_composer/sqlite.rs b/crates/tracedecay-sessions/src/runtime/hosts/cursor_composer/sqlite.rs index c96205b831..e0821235fa 100644 --- a/crates/tracedecay-sessions/src/runtime/hosts/cursor_composer/sqlite.rs +++ b/crates/tracedecay-sessions/src/runtime/hosts/cursor_composer/sqlite.rs @@ -114,11 +114,11 @@ pub(super) async fn open_readonly_immutable(db_path: &Path) -> Result