fix(storage): preserve SQLite locks during pathname identity checks

crates/tracedecay-rusqlite-runtime/src/connection/mod.rs

@@ -201,8 +201,22 @@ impl OpenedDatabaseFile {
     }
 
     pub(crate) fn verify_current_path(&self, path: &Path) -> Result<(), OpenedDatabaseFileError> {
-        let current = File::open(path).map_err(|_| OpenedDatabaseFileError::Open)?;
-        if opened_file_identity(&current)? != self.identity {
+        // POSIX record locks belong to the process: closing any descriptor
+        // for this inode releases SQLite's locks, even on another connection.
+        // Inspect the pathname without opening a temporary descriptor. Keep
+        // using the pinned descriptor's dev/inode identity, and retain both
+        // checks around SQLITE_FCNTL_HAS_MOVED in verify_connection.
+        #[cfg(unix)]
+        let current_identity = {
+            let current = std::fs::metadata(path).map_err(|_| OpenedDatabaseFileError::Open)?;
+            metadata_identity(&current)
+        };
+        #[cfg(not(unix))]
+        let current_identity = {
+            let current = File::open(path).map_err(|_| OpenedDatabaseFileError::Open)?;
+            opened_file_identity(&current)?
+        };
+        if current_identity != self.identity {
             return Err(OpenedDatabaseFileError::Replaced);
         }
         Ok(())
@@ -276,6 +290,25 @@ impl OpenedDatabaseFile {
     }
 }
 
+/// Check a live SQLite worker's physical file without opening and closing a
+/// second descriptor for the same inode on Unix. Closing that descriptor can
+/// release process-wide SQLite locks owned by other connections, even though
+/// SQLite's VFS did not perform the close.
+pub(crate) fn live_path_identity(path: &Path) -> Result<u64, OpenedDatabaseFileError> {
+    #[cfg(unix)]
+    {
+        let metadata = std::fs::metadata(path).map_err(|_| OpenedDatabaseFileError::Open)?;
+        if !metadata.is_file() {
+            return Err(OpenedDatabaseFileError::NotFile);
+        }
+        Ok(metadata_identity(&metadata))
+    }
+    #[cfg(not(unix))]
+    {
+        OpenedDatabaseFile::pin(path).map(|opened| opened.identity())
+    }
+}
+
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum OpenedDatabaseFileError {
     Create,
@@ -1269,5 +1302,8 @@ fn policy(stage: &'static str, source: rusqlite::Error) -> ConnectionPolicyError
     ConnectionPolicyError { stage, source }
 }
 
+#[cfg(all(test, unix))]
+mod path_identity_tests;
+
 #[cfg(test)]
 mod tests;

crates/tracedecay-rusqlite-runtime/src/connection/path_identity_tests.rs

@@ -0,0 +1,242 @@
+use std::{env, fs, path::Path, process::Command, time::Duration};
+
+use rusqlite::{Connection, Error, ErrorCode, OpenFlags};
+use tempfile::TempDir;
+
+use super::{OpenedDatabaseFile, OpenedDatabaseFileError, live_path_identity};
+
+const PROBE_PATH: &str = "TRACEDECAY_PATH_IDENTITY_PROBE_PATH";
+const PROBE_OPERATION: &str = "TRACEDECAY_PATH_IDENTITY_PROBE_OPERATION";
+const PROBE_EXPECTATION: &str = "TRACEDECAY_PATH_IDENTITY_PROBE_EXPECTATION";
+const PROBE_COMPLETE: &str = "tracedecay-path-identity-probe-complete";
+
+// A second connection in this process is not a lock probe: SQLite's Unix VFS
+// shares inode/lock bookkeeping within a process and can hide a lost OS lock.
+// Execute only the child test in a fresh process, with a marker to reject a
+// successful test harness invocation that accidentally selected zero tests.
+fn probe_external_connection(path: &Path, operation: &str, blocked: bool) {
+    let (_, module) = module_path!().split_once("::").expect("test module path");
+    let output = Command::new(env::current_exe().expect("test executable"))
+        .args([
+            "--exact",
+            &format!("{module}::external_connection_probe"),
+            "--nocapture",
+        ])
+        .env(PROBE_PATH, path)
+        .env(PROBE_OPERATION, operation)
+        .env(
+            PROBE_EXPECTATION,
+            if blocked { "blocked" } else { "allowed" },
+        )
+        .output()
+        .expect("run external SQLite connection");
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    assert!(
+        output.status.success() && stdout.contains(PROBE_COMPLETE),
+        "external {operation} probe failed: {}\n{stdout}\n{stderr}",
+        output.status,
+    );
+}
+
+#[test]
+fn external_connection_probe() {
+    let Some(path) = env::var_os(PROBE_PATH) else {
+        return;
+    };
+    let operation = env::var(PROBE_OPERATION).expect("probe operation");
+    let expectation = env::var(PROBE_EXPECTATION).expect("probe expectation");
+    let connection = Connection::open_with_flags(
+        Path::new(&path),
+        OpenFlags::SQLITE_OPEN_READ_WRITE | OpenFlags::SQLITE_OPEN_NO_MUTEX,
+    )
+    .expect("open existing database in another process");
+    connection
+        .busy_timeout(Duration::ZERO)
+        .expect("no lock wait");
+    let result = match operation.as_str() {
+        "write" => connection.execute_batch("BEGIN IMMEDIATE; ROLLBACK"),
+        "journal" => connection
+            .query_row("PRAGMA journal_mode = DELETE", [], |row| {
+                row.get::<_, String>(0)
+            })
+            .map(|mode| assert_eq!(mode, "delete")),
+        _ => panic!("unknown probe operation: {operation}"),
+    };
+    match expectation.as_str() {
+        "blocked" => assert!(
+            matches!(
+                &result,
+                Err(Error::SqliteFailure(error, _)) if error.code == ErrorCode::DatabaseBusy
+            ),
+            "expected SQLITE_BUSY from external {operation}, got {result:?}",
+        ),
+        "allowed" => result.expect("external operation after lock release"),
+        _ => panic!("unknown probe expectation: {expectation}"),
+    }
+    println!("{PROBE_COMPLETE}");
+}
+
+fn assert_committed_data(path: &Path) {
+    let connection = Connection::open_with_flags(path, OpenFlags::SQLITE_OPEN_READ_ONLY)
+        .expect("reopen committed database");
+    let value: i64 = connection
+        .query_row("SELECT value FROM sample", [], |row| row.get(0))
+        .expect("committed value");
+    assert_eq!(value, 2);
+    let integrity: String = connection
+        .query_row("PRAGMA integrity_check", [], |row| row.get(0))
+        .expect("integrity check");
+    assert_eq!(integrity, "ok");
+}
+
+#[test]
+fn worker_identity_checks_keep_cross_process_writer_exclusion() {
+    let temp = TempDir::new().expect("tempdir");
+    let path = temp.path().join("worker.db");
+    let connection = Connection::open(&path).expect("open database");
+    connection
+        .execute_batch(
+            "PRAGMA journal_mode = DELETE;
+             CREATE TABLE sample(value INTEGER);
+             INSERT INTO sample VALUES (1);
+             BEGIN IMMEDIATE;
+             UPDATE sample SET value = 2;",
+        )
+        .expect("hold live SQLite writer lock");
+    // The old worker check opened another raw descriptor and dropped it
+    // immediately. On POSIX that released this transaction's advisory lock.
+    probe_external_connection(&path, "write", true);
+    let identity = live_path_identity(&path).expect("inspect live worker identity");
+    assert_ne!(identity, 0);
+    probe_external_connection(&path, "write", true);
+    assert_eq!(live_path_identity(&path).unwrap(), identity);
+    probe_external_connection(&path, "write", true);
+
+    connection.execute_batch("COMMIT").expect("commit");
+    probe_external_connection(&path, "write", false);
+    drop(connection);
+    assert_committed_data(&path);
+
+    let absent = temp.path().join("absent.db");
+    assert_eq!(
+        live_path_identity(&absent),
+        Err(OpenedDatabaseFileError::Open)
+    );
+    let directory = temp.path().join("not-a-database");
+    fs::create_dir(&directory).expect("directory");
+    assert_eq!(
+        live_path_identity(&directory),
+        Err(OpenedDatabaseFileError::NotFile)
+    );
+}
+
+#[test]
+fn path_checks_preserve_rollback_writer_lock() {
+    let temp = TempDir::new().expect("tempdir");
+    let path = temp.path().join("rollback.db");
+    // Pin before SQLite takes locks, and keep the pin until after it closes.
+    // This isolates pathname verification from the separate pin-drop lifetime.
+    let pinned = OpenedDatabaseFile::create_new(&path).expect("pin database");
+    let connection = Connection::open(&path).expect("open database");
+    connection
+        .execute_batch(
+            "PRAGMA journal_mode = DELETE;
+             CREATE TABLE sample(value INTEGER);
+             INSERT INTO sample VALUES (1);
+             BEGIN IMMEDIATE;
+             UPDATE sample SET value = 2;",
+        )
+        .expect("hold write transaction");
+    probe_external_connection(&path, "write", true);
+
+    pinned.verify_current_path(&path).expect("same inode");
+    probe_external_connection(&path, "write", true);
+    pinned
+        .verify_connection(&connection, &path)
+        .expect("same SQLite connection");
+    probe_external_connection(&path, "write", true);
+
+    connection.execute_batch("COMMIT").expect("commit");
+    probe_external_connection(&path, "write", false);
+    drop(connection);
+    drop(pinned);
+    assert_committed_data(&path);
+}
+
+#[test]
+fn path_checks_preserve_live_wal_journal_mode_guard() {
+    let temp = TempDir::new().expect("tempdir");
+    let path = temp.path().join("wal.db");
+    let pinned = OpenedDatabaseFile::create_new(&path).expect("pin database");
+    let connection = Connection::open(&path).expect("open database");
+    connection
+        .execute_batch(
+            "PRAGMA journal_mode = WAL;
+             CREATE TABLE sample(value INTEGER);
+             INSERT INTO sample VALUES (1);
+             BEGIN IMMEDIATE;
+             UPDATE sample SET value = 2;",
+        )
+        .expect("hold WAL write transaction");
+    probe_external_connection(&path, "journal", true);
+
+    pinned.verify_current_path(&path).expect("same inode");
+    probe_external_connection(&path, "journal", true);
+    pinned
+        .verify_connection(&connection, &path)
+        .expect("same SQLite connection");
+    probe_external_connection(&path, "journal", true);
+
+    connection.execute_batch("COMMIT").expect("commit");
+    // A live WAL connection still prevents another process switching modes,
+    // even between transactions. Only closing the connection releases it.
+    probe_external_connection(&path, "journal", true);
+    drop(connection);
+    probe_external_connection(&path, "journal", false);
+    drop(pinned);
+    assert_committed_data(&path);
+}
+
+#[test]
+fn path_checks_keep_pinned_identity_and_reject_missing_or_replaced_files() {
+    let temp = TempDir::new().expect("tempdir");
+    let path = temp.path().join("identity.db");
+    let alias = temp.path().join("same-inode.db");
+    let symlink = temp.path().join("symlink.db");
+    let pinned = OpenedDatabaseFile::create_new(&path).expect("pin database");
+    fs::hard_link(&path, &alias).expect("hard link");
+    std::os::unix::fs::symlink(&path, &symlink).expect("symlink");
+    pinned.verify_current_path(&path).expect("original inode");
+    pinned.verify_current_path(&alias).expect("hard-link inode");
+    pinned
+        .verify_current_path(&symlink)
+        .expect("symlink target");
+
+    fs::remove_file(&path).expect("unlink original path");
+    assert_eq!(
+        pinned.verify_current_path(&path),
+        Err(OpenedDatabaseFileError::Open),
+    );
+    assert_eq!(
+        pinned.verify_current_path(&symlink),
+        Err(OpenedDatabaseFileError::Open),
+    );
+    fs::write(&path, b"").expect("create different inode");
+    assert_eq!(
+        pinned.verify_current_path(&path),
+        Err(OpenedDatabaseFileError::Replaced),
+    );
+    assert_eq!(
+        pinned.verify_current_path(&symlink),
+        Err(OpenedDatabaseFileError::Replaced),
+    );
+    pinned.verify_current_path(&alias).expect("retained inode");
+
+    fs::remove_file(&path).expect("remove replacement");
+    fs::create_dir(&path).expect("directory at database path");
+    assert_eq!(
+        pinned.verify_current_path(&path),
+        Err(OpenedDatabaseFileError::Replaced),
+    );
+}

crates/tracedecay-rusqlite-runtime/src/reader/worker.rs

@@ -18,7 +18,7 @@ use tracedecay_store::{
     StorageRuntimeErrorV1, UnavailableReasonV1,
 };
 
-use crate::connection::{self, ConnectionMode, OpenedDatabaseFile};
+use crate::connection::{self, ConnectionMode};
 use crate::exact_sql::{
     ExactSqlError, ExactSqlRows, ExactSqlStatement, ExactSqlStatementBudget, execute_query,
     read_across_wal_recovery,
@@ -516,8 +516,8 @@ pub(crate) fn spawn<E: ReaderQueryExecutor>(
             let _ = started.send(Err(error));
             return;
         }
-        let opened_file_identity = match OpenedDatabaseFile::pin(&worker_open_path) {
-            Ok(opened) => opened.identity(),
+        let opened_file_identity = match connection::live_path_identity(&worker_open_path) {
+            Ok(identity) => identity,
             Err(error) => {
                 let _ = started.send(Err(ReaderStartError::OpenedDatabaseIdentity(error)));
                 return;

crates/tracedecay-rusqlite-runtime/src/writer/worker/mod.rs

@@ -434,8 +434,8 @@ impl Worker {
         }
         let opened_file_identity = match self.expected_file_identity {
             Some(expected) => {
-                let actual = match OpenedDatabaseFile::pin(&self.path) {
-                    Ok(opened) => opened.identity(),
+                let actual = match connection::live_path_identity(&self.path) {
+                    Ok(identity) => identity,
                     Err(error) => {
                         return self.fail_start(WriterStartError::OpenedDatabaseIdentity(error));
                     }