From 2a29717c5ac7e7194e0a02d8dea9461a092fd13e Mon Sep 17 00:00:00 2001 From: gwenn Date: Sat, 11 Aug 2018 11:14:17 +0200 Subject: [PATCH] Fix hooks --- src/functions.rs | 2 +- src/hooks.rs | 72 ++++++++++++++++++++++++++++++++++++------------ src/lib.rs | 19 +++++++++++-- 3 files changed, 73 insertions(+), 20 deletions(-) diff --git a/src/functions.rs b/src/functions.rs index 91aecf2..10db830 100644 --- a/src/functions.rs +++ b/src/functions.rs @@ -182,7 +182,7 @@ impl<'a> ValueRef<'a> { } unsafe extern "C" fn free_boxed_value(p: *mut c_void) { - let _: Box = Box::from_raw(p as *mut T); + drop(Box::from_raw(p as *mut T)); } /// Context is a wrapper for the SQLite function evaluation context. diff --git a/src/hooks.rs b/src/hooks.rs index 541f084..b3e2fa6 100644 --- a/src/hooks.rs +++ b/src/hooks.rs @@ -1,7 +1,6 @@ //! Commit, Data Change and Rollback Notification Callbacks #![allow(non_camel_case_types)] -use std::mem::size_of; use std::ptr; use std::os::raw::{c_int, c_char, c_void}; @@ -136,12 +135,10 @@ impl InnerConnection { self.rollback_hook(None::); } - fn commit_hook(&self, hook: Option) + fn commit_hook(&mut self, hook: Option) where F: FnMut() -> bool, { - assert_eq!(size_of::<*mut F>(), size_of::<*mut c_void>()); - unsafe extern "C" fn call_boxed_closure(p_arg: *mut c_void) -> c_int where F: FnMut() -> bool, @@ -154,6 +151,14 @@ impl InnerConnection { } } + // unlike `sqlite3_create_function_v2`, we cannot specify a `xDestroy` with `sqlite3_commit_hook`. + // so we keep the `xDestroy` function in `InnerConnection.free_boxed_hook`. + let free_commit_hook = if hook.is_some() { + Some(free_boxed_hook:: as fn(*mut c_void)) + } else { + None + }; + let previous_hook = match hook { Some(hook) => { let boxed_hook: *mut F = Box::into_raw(Box::new(hook)); @@ -167,15 +172,18 @@ impl InnerConnection { } _ => unsafe { ffi::sqlite3_commit_hook(self.db(), None, ptr::null_mut()) }, }; - free_boxed_hook(previous_hook); + if !previous_hook.is_null() { + if let Some(free_boxed_hook) = self.free_commit_hook { + free_boxed_hook(previous_hook); + } + } + self.free_commit_hook = free_commit_hook; } - fn rollback_hook(&self, hook: Option) + fn rollback_hook(&mut self, hook: Option) where F: FnMut(), { - assert_eq!(size_of::<*mut F>(), size_of::<*mut c_void>()); - unsafe extern "C" fn call_boxed_closure(p_arg: *mut c_void) where F: FnMut(), @@ -184,6 +192,12 @@ impl InnerConnection { (*boxed_hook)(); } + let free_rollback_hook = if hook.is_some() { + Some(free_boxed_hook:: as fn(*mut c_void)) + } else { + None + }; + let previous_hook = match hook { Some(hook) => { let boxed_hook: *mut F = Box::into_raw(Box::new(hook)); @@ -197,15 +211,18 @@ impl InnerConnection { } _ => unsafe { ffi::sqlite3_rollback_hook(self.db(), None, ptr::null_mut()) }, }; - free_boxed_hook(previous_hook); + if !previous_hook.is_null() { + if let Some(free_boxed_hook) = self.free_rollback_hook { + free_boxed_hook(previous_hook); + } + } + self.free_rollback_hook = free_rollback_hook; } fn update_hook(&mut self, hook: Option) where F: FnMut(Action, &str, &str, i64), { - assert_eq!(size_of::<*mut F>(), size_of::<*mut c_void>()); - unsafe extern "C" fn call_boxed_closure( p_arg: *mut c_void, action_code: c_int, @@ -233,6 +250,12 @@ impl InnerConnection { (*boxed_hook)(action, db_name, tbl_name, row_id); } + let free_update_hook = if hook.is_some() { + Some(free_boxed_hook:: as fn(*mut c_void)) + } else { + None + }; + let previous_hook = match hook { Some(hook) => { let boxed_hook: *mut F = Box::into_raw(Box::new(hook)); @@ -246,15 +269,17 @@ impl InnerConnection { } _ => unsafe { ffi::sqlite3_update_hook(self.db(), None, ptr::null_mut()) }, }; - free_boxed_hook(previous_hook); + if !previous_hook.is_null() { + if let Some(free_boxed_hook) = self.free_update_hook { + free_boxed_hook(previous_hook); + } + } + self.free_update_hook = free_update_hook; } } -fn free_boxed_hook(hook: *mut c_void) { - if !hook.is_null() { - // make sure that size_of::<*mut F>() is always equal to size_of::<*mut c_void>() - let _: Box<*mut c_void> = unsafe { Box::from_raw(hook as *mut _) }; - } +fn free_boxed_hook(p: *mut c_void) { + drop(unsafe { Box::from_raw(p as *mut F) }); } #[cfg(test)] @@ -276,6 +301,19 @@ mod test { assert!(called); } + #[test] + fn test_fn_commit_hook() { + let db = Connection::open_in_memory().unwrap(); + + fn hook() -> bool { + true + } + + db.commit_hook(Some(hook)); + db.execute_batch("BEGIN; CREATE TABLE foo (t TEXT); COMMIT;") + .unwrap_err(); + } + #[test] fn test_rollback_hook() { let db = Connection::open_in_memory().unwrap(); diff --git a/src/lib.rs b/src/lib.rs index 598520b..eb5bf4b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -72,7 +72,7 @@ use std::result; use std::str; use std::sync::{Once, ONCE_INIT}; use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering}; -use std::os::raw::{c_int, c_char}; +use std::os::raw::{c_int, c_char, c_void}; use types::{ToSql, ValueRef}; use error::{error_from_sqlite_code, error_from_handle}; @@ -597,6 +597,12 @@ impl fmt::Debug for Connection { struct InnerConnection { db: *mut ffi::sqlite3, + #[cfg(feature = "hooks")] + free_commit_hook: Option, + #[cfg(feature = "hooks")] + free_rollback_hook: Option, + #[cfg(feature = "hooks")] + free_update_hook: Option, } /// Old name for `OpenFlags`. `SqliteOpenFlags` is deprecated. @@ -755,6 +761,15 @@ To fix this, either: } impl InnerConnection { + #[cfg(not(feature = "hooks"))] + fn new(db: *mut ffi::sqlite3) -> InnerConnection { + InnerConnection { db } + } + #[cfg(feature = "hooks")] + fn new(db: *mut ffi::sqlite3) -> InnerConnection { + InnerConnection { db, free_commit_hook: None, free_rollback_hook: None, free_update_hook: None } + } + fn open_with_flags(c_path: &CString, flags: OpenFlags) -> Result { ensure_valid_sqlite_version(); ensure_safe_sqlite_threading_mode()?; @@ -792,7 +807,7 @@ impl InnerConnection { // attempt to turn on extended results code; don't fail if we can't. ffi::sqlite3_extended_result_codes(db, 1); - Ok(InnerConnection { db }) + Ok(InnerConnection::new(db)) } }