Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add progress handler support to sqlite #2256

Merged
merged 6 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sqlx-sqlite/src/connection/establish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ impl EstablishParams {
statements: Statements::new(self.statement_cache_capacity),
transaction_depth: 0,
log_settings: self.log_settings.clone(),
progress_handler_callback: None,
})
}
}
79 changes: 78 additions & 1 deletion sqlx-sqlite/src/connection/mod.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use futures_core::future::BoxFuture;
use futures_intrusive::sync::MutexGuard;
use futures_util::future;
use libsqlite3_sys::sqlite3;
use libsqlite3_sys::{sqlite3, sqlite3_progress_handler};
use sqlx_core::common::StatementCache;
use sqlx_core::error::Error;
use sqlx_core::transaction::Transaction;
use std::cmp::Ordering;
use std::fmt::{self, Debug, Formatter};
use std::os::raw::{c_int, c_void};
use std::panic::catch_unwind;
use std::ptr::NonNull;

use crate::connection::establish::EstablishParams;
Expand Down Expand Up @@ -51,6 +53,10 @@ pub struct LockedSqliteHandle<'a> {
pub(crate) guard: MutexGuard<'a, ConnectionState>,
}

/// Represents a callback handler that will be shared with the underlying sqlite3 connection.
pub(crate) struct Handler(NonNull<dyn FnMut() -> bool + Send>);
unsafe impl Send for Handler {}

pub(crate) struct ConnectionState {
pub(crate) handle: ConnectionHandle,

Expand All @@ -60,6 +66,22 @@ pub(crate) struct ConnectionState {
pub(crate) statements: Statements,

log_settings: LogSettings,

/// Stores the progress handler set on the current connection. If the handler returns `false`,
/// the query is interrupted.
progress_handler_callback: Option<Handler>,
}

impl ConnectionState {
/// Drops the `progress_handler_callback` if it exists.
pub(crate) fn remove_progress_handler(&mut self) {
if let Some(mut handler) = self.progress_handler_callback.take() {
unsafe {
sqlite3_progress_handler(self.handle.as_ptr(), 0, None, 0 as *mut _);
let _ = { Box::from_raw(handler.0.as_mut()) };
}
}
}
}

pub(crate) struct Statements {
Expand Down Expand Up @@ -172,6 +194,21 @@ impl Connection for SqliteConnection {
}
}

/// Implements a C binding to a progress callback. The function returns `0` if the
/// user-provided callback returns `true`, and `1` otherwise to signal an interrupt.
extern "C" fn progress_callback<F>(callback: *mut c_void) -> c_int
where
F: FnMut() -> bool,
{
unsafe {
let r = catch_unwind(|| {
let callback: *mut F = callback.cast::<F>();
(*callback)()
});
c_int::from(!r.unwrap_or_default())
}
}

impl LockedSqliteHandle<'_> {
/// Returns the underlying sqlite3* connection handle.
///
Expand Down Expand Up @@ -201,12 +238,52 @@ impl LockedSqliteHandle<'_> {
) -> Result<(), Error> {
collation::create_collation(&mut self.guard.handle, name, compare)
}

/// Sets a progress handler that is invoked periodically during long running calls. If the progress callback
/// returns `false`, then the operation is interrupted.
///
/// `num_ops` is the approximate number of [virtual machine instructions](https://www.sqlite.org/opcode.html)
/// that are evaluated between successive invocations of the callback. If `num_ops` is less than one then the
/// progress handler is disabled.
///
/// Only a single progress handler may be defined at one time per database connection; setting a new progress
/// handler cancels the old one.
///
/// The progress handler callback must not do anything that will modify the database connection that invoked
/// the progress handler. Note that sqlite3_prepare_v2() and sqlite3_step() both modify their database connections
/// in this context.
pub fn set_progress_handler<F>(&mut self, num_ops: i32, mut callback: F)
where
F: FnMut() -> bool + Send + 'static,
{
unsafe {
let callback_boxed = Box::new(callback);
// SAFETY: `Box::into_raw()` always returns a non-null pointer.
let callback = NonNull::new_unchecked(Box::into_raw(callback_boxed));
let handler = callback.as_ptr() as *mut _;
self.guard.remove_progress_handler();
self.guard.progress_handler_callback = Some(Handler(callback));

sqlite3_progress_handler(
self.as_raw_handle().as_mut(),
num_ops,
Some(progress_callback::<F>),
handler,
);
}
}

/// Removes the progress handler on a database connection. The method does nothing if no handler was set.
pub fn remove_progress_handler(&mut self) {
self.guard.remove_progress_handler();
}
}

impl Drop for ConnectionState {
fn drop(&mut self) {
// explicitly drop statements before the connection handle is dropped
self.statements.clear();
self.remove_progress_handler();
}
}

Expand Down
84 changes: 84 additions & 0 deletions tests/sqlite/sqlite.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![feature(unboxed_closures)]
#![feature(fn_traits)]
nbaztec marked this conversation as resolved.
Show resolved Hide resolved

use futures::TryStreamExt;
use rand::{Rng, SeedableRng};
use rand_xoshiro::Xoshiro256PlusPlus;
Expand All @@ -7,6 +10,7 @@ use sqlx::{
SqliteConnection, SqlitePool, Statement, TypeInfo,
};
use sqlx_test::new;
use std::sync::atomic::{AtomicUsize, Ordering};

#[sqlx_macros::test]
async fn it_connects() -> anyhow::Result<()> {
Expand Down Expand Up @@ -725,3 +729,83 @@ async fn concurrent_read_and_write() {
read.await;
write.await;
}

#[sqlx_macros::test]
async fn test_query_with_progress_handler() -> anyhow::Result<()> {
let mut conn = new::<Sqlite>().await?;

// Using this string as a canary to ensure the callback doesn't get called with the wrong data pointer.
let state = format!("test");
conn.lock_handle().await?.set_progress_handler(1, move || {
assert_eq!(state, "test");
false
});

match sqlx::query("SELECT 'hello' AS title")
.fetch_all(&mut conn)
.await
{
Err(sqlx::Error::Database(err)) => assert_eq!(err.message(), String::from("interrupted")),
_ => panic!("expected an interrupt"),
}

Ok(())
}

#[sqlx_macros::test]
async fn test_multiple_set_progress_handler_calls_drop_old_handler() -> anyhow::Result<()> {
static OBJECTS_DROPPED: AtomicUsize = AtomicUsize::new(0);

struct Handler(pub &'static str);
impl FnOnce<()> for Handler {
type Output = bool;

extern "rust-call" fn call_once(mut self, args: ()) -> bool {
self.call_mut(args)
}
}
impl FnMut<()> for Handler {
extern "rust-call" fn call_mut(&mut self, _args: ()) -> bool {
assert_eq!(3, self.0.len());
false
}
}
impl Drop for Handler {
fn drop(&mut self) {
OBJECTS_DROPPED.fetch_add(1, Ordering::Relaxed);
}
}
nbaztec marked this conversation as resolved.
Show resolved Hide resolved

{
let mut conn = new::<Sqlite>().await?;

conn.lock_handle()
.await?
.set_progress_handler(1, Handler("foo"));
conn.lock_handle()
.await?
.set_progress_handler(1, Handler("bar"));
conn.lock_handle()
.await?
.set_progress_handler(1, Handler("baz"));

match sqlx::query("SELECT 'hello' AS title")
.fetch_all(&mut conn)
.await
{
Err(sqlx::Error::Database(err)) => {
assert_eq!(err.message(), String::from("interrupted"))
}
_ => panic!("expected an interrupt"),
}

conn.lock_handle().await?.remove_progress_handler();
}

assert_eq!(
3,
OBJECTS_DROPPED.load(Ordering::Relaxed),
"expected all handlers to be dropped"
);
Ok(())
}