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