From 519684a7446009f8d0ab0da3feea8de1a06b15d8 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Tue, 4 Jan 2022 20:23:32 -0800 Subject: [PATCH] cleanup unlock_notify code a bit --- src/inner_connection.rs | 47 +++++++++++++++++++++-------------------- src/lib.rs | 1 + src/raw_statement.rs | 40 +++++++++++++++++++++++------------ src/unlock_notify.rs | 29 +++++++------------------ 4 files changed, 59 insertions(+), 58 deletions(-) diff --git a/src/inner_connection.rs b/src/inner_connection.rs index d03d04a..b3b1bcd 100644 --- a/src/inner_connection.rs +++ b/src/inner_connection.rs @@ -13,7 +13,6 @@ use super::{Connection, InterruptHandle, OpenFlags, Result}; use crate::error::{error_from_handle, error_from_sqlite_code, Error}; use crate::raw_statement::RawStatement; use crate::statement::Statement; -use crate::unlock_notify; use crate::version::version_number; pub struct InnerConnection { @@ -223,35 +222,37 @@ impl InnerConnection { let mut c_stmt = ptr::null_mut(); let (c_sql, len, _) = str_for_sqlite(sql.as_bytes())?; let mut c_tail = ptr::null(); + #[cfg(not(feature = "unlock_notify"))] let r = unsafe { - if cfg!(feature = "unlock_notify") { - let mut rc; - loop { - rc = ffi::sqlite3_prepare_v2( - self.db(), - c_sql, - len, - &mut c_stmt as *mut *mut ffi::sqlite3_stmt, - &mut c_tail as *mut *const c_char, - ); - if !unlock_notify::is_locked(self.db, rc) { - break; - } - rc = unlock_notify::wait_for_unlock_notify(self.db); - if rc != ffi::SQLITE_OK { - break; - } - } - rc - } else { - ffi::sqlite3_prepare_v2( + ffi::sqlite3_prepare_v2( + self.db(), + c_sql, + len, + &mut c_stmt as *mut *mut ffi::sqlite3_stmt, + &mut c_tail as *mut *const c_char, + ) + }; + #[cfg(feature = "unlock_notify")] + let r = unsafe { + use crate::unlock_notify; + let mut rc; + loop { + rc = ffi::sqlite3_prepare_v2( self.db(), c_sql, len, &mut c_stmt as *mut *mut ffi::sqlite3_stmt, &mut c_tail as *mut *const c_char, - ) + ); + if !unlock_notify::is_locked(self.db, rc) { + break; + } + rc = unlock_notify::wait_for_unlock_notify(self.db); + if rc != ffi::SQLITE_OK { + break; + } } + rc }; // If there is an error, *ppStmt is set to NULL. self.decode_result(r)?; diff --git a/src/lib.rs b/src/lib.rs index 531b37f..5a5e881 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -125,6 +125,7 @@ mod statement; pub mod trace; mod transaction; pub mod types; +#[cfg(feature = "unlock_notify")] mod unlock_notify; mod version; #[cfg(feature = "vtab")] diff --git a/src/raw_statement.rs b/src/raw_statement.rs index ef94d81..787edb1 100644 --- a/src/raw_statement.rs +++ b/src/raw_statement.rs @@ -1,5 +1,4 @@ use super::ffi; -use super::unlock_notify; use super::StatementStatus; #[cfg(feature = "modern_sqlite")] use crate::util::SqliteMallocString; @@ -101,25 +100,38 @@ impl RawStatement { } } - #[cfg_attr(not(feature = "unlock_notify"), inline)] + #[inline] + #[cfg(not(feature = "unlock_notify"))] pub fn step(&self) -> c_int { - if cfg!(feature = "unlock_notify") { - let db = unsafe { ffi::sqlite3_db_handle(self.ptr) }; - let mut rc; - loop { - rc = unsafe { ffi::sqlite3_step(self.ptr) }; - if unsafe { !unlock_notify::is_locked(db, rc) } { - break; + unsafe { ffi::sqlite3_step(self.ptr) } + } + + #[cfg(feature = "unlock_notify")] + pub fn step(&self) -> c_int { + use crate::unlock_notify; + let mut db = core::ptr::null_mut::(); + loop { + unsafe { + let mut rc = ffi::sqlite3_step(self.ptr); + // Bail out early for success and errors unrelated to locking. We + // still need check `is_locked` after this, but checking now lets us + // avoid one or two (admittedly cheap) calls into SQLite that we + // don't need to make. + if (rc & 0xff) != ffi::SQLITE_LOCKED { + break rc; } - rc = unsafe { unlock_notify::wait_for_unlock_notify(db) }; + if db.is_null() { + db = ffi::sqlite3_db_handle(self.ptr); + } + if !unlock_notify::is_locked(db, rc) { + break rc; + } + rc = unlock_notify::wait_for_unlock_notify(db); if rc != ffi::SQLITE_OK { - break; + break rc; } self.reset(); } - rc - } else { - unsafe { ffi::sqlite3_step(self.ptr) } } } diff --git a/src/unlock_notify.rs b/src/unlock_notify.rs index 5f9cdbf..8bb397d 100644 --- a/src/unlock_notify.rs +++ b/src/unlock_notify.rs @@ -1,22 +1,17 @@ //! [Unlock Notification](http://sqlite.org/unlock_notify.html) use std::os::raw::c_int; -#[cfg(feature = "unlock_notify")] use std::os::raw::c_void; -#[cfg(feature = "unlock_notify")] use std::panic::catch_unwind; -#[cfg(feature = "unlock_notify")] use std::sync::{Condvar, Mutex}; use crate::ffi; -#[cfg(feature = "unlock_notify")] struct UnlockNotification { cond: Condvar, // Condition variable to wait on mutex: Mutex, // Mutex to protect structure } -#[cfg(feature = "unlock_notify")] #[allow(clippy::mutex_atomic)] impl UnlockNotification { fn new() -> UnlockNotification { @@ -27,21 +22,25 @@ impl UnlockNotification { } fn fired(&self) { - let mut flag = self.mutex.lock().unwrap(); + let mut flag = unpoison(self.mutex.lock()); *flag = true; self.cond.notify_one(); } fn wait(&self) { - let mut fired = self.mutex.lock().unwrap(); + let mut fired = unpoison(self.mutex.lock()); while !*fired { - fired = self.cond.wait(fired).unwrap(); + fired = unpoison(self.cond.wait(fired)); } } } +#[inline] +fn unpoison(r: Result>) -> T { + r.unwrap_or_else(std::sync::PoisonError::into_inner) +} + /// This function is an unlock-notify callback -#[cfg(feature = "unlock_notify")] unsafe extern "C" fn unlock_notify_cb(ap_arg: *mut *mut c_void, n_arg: c_int) { use std::slice::from_raw_parts; let args = from_raw_parts(ap_arg as *const &UnlockNotification, n_arg as usize); @@ -50,7 +49,6 @@ unsafe extern "C" fn unlock_notify_cb(ap_arg: *mut *mut c_void, n_arg: c_int) { } } -#[cfg(feature = "unlock_notify")] pub unsafe fn is_locked(db: *mut ffi::sqlite3, rc: c_int) -> bool { rc == ffi::SQLITE_LOCKED_SHAREDCACHE || (rc & 0xFF) == ffi::SQLITE_LOCKED @@ -87,17 +85,6 @@ pub unsafe fn wait_for_unlock_notify(db: *mut ffi::sqlite3) -> c_int { rc } -#[cfg(not(feature = "unlock_notify"))] -pub unsafe fn is_locked(_db: *mut ffi::sqlite3, _rc: c_int) -> bool { - unreachable!() -} - -#[cfg(not(feature = "unlock_notify"))] -pub unsafe fn wait_for_unlock_notify(_db: *mut ffi::sqlite3) -> c_int { - unreachable!() -} - -#[cfg(feature = "unlock_notify")] #[cfg(test)] mod test { use crate::{Connection, OpenFlags, Result, Transaction, TransactionBehavior};