From 0e482baba9187f08f9c0d1dc4dcf1c3a00eb8f2c Mon Sep 17 00:00:00 2001 From: Austin Bonander Date: Wed, 22 Sep 2021 15:49:16 -0700 Subject: [PATCH] fix(sqlite): hold a reference to the connection in `SqliteRow` fixes #1419 --- sqlx-core/src/sqlite/connection/executor.rs | 9 ++++-- sqlx-core/src/sqlite/connection/mod.rs | 3 +- sqlx-core/src/sqlite/row.rs | 8 +++--- sqlx-core/src/sqlite/statement/handle.rs | 32 +++++++++++++++++++++ sqlx-core/src/sqlite/statement/mod.rs | 2 +- 5 files changed, 44 insertions(+), 10 deletions(-) diff --git a/sqlx-core/src/sqlite/connection/executor.rs b/sqlx-core/src/sqlite/connection/executor.rs index 09bbc6cf5b..2998e38352 100644 --- a/sqlx-core/src/sqlite/connection/executor.rs +++ b/sqlx-core/src/sqlite/connection/executor.rs @@ -152,7 +152,7 @@ impl<'c> Executor<'c> for &'c mut SqliteConnection { Either::Right(()) => { let (row, weak_values_ref) = SqliteRow::current( - &stmt, + stmt.to_ref(self.handle.to_ref()), columns, column_names ); @@ -216,8 +216,11 @@ impl<'c> Executor<'c> for &'c mut SqliteConnection { Either::Left(_) => (), Either::Right(()) => { - let (row, weak_values_ref) = - SqliteRow::current(stmt, columns, column_names); + let (row, weak_values_ref) = SqliteRow::current( + stmt.to_ref(self.handle.to_ref()), + columns, + column_names, + ); *last_row_values = Some(weak_values_ref); diff --git a/sqlx-core/src/sqlite/connection/mod.rs b/sqlx-core/src/sqlite/connection/mod.rs index c18add85b2..4edb9d93bc 100644 --- a/sqlx-core/src/sqlite/connection/mod.rs +++ b/sqlx-core/src/sqlite/connection/mod.rs @@ -104,8 +104,7 @@ impl Connection for SqliteConnection { impl Drop for SqliteConnection { fn drop(&mut self) { - // before the connection handle is dropped, - // we must explicitly drop the statements as the drop-order in a struct is undefined + // explicitly drop statements before the connection handle is dropped self.statements.clear(); self.statement.take(); } diff --git a/sqlx-core/src/sqlite/row.rs b/sqlx-core/src/sqlite/row.rs index f1e15f599d..4199915fe1 100644 --- a/sqlx-core/src/sqlite/row.rs +++ b/sqlx-core/src/sqlite/row.rs @@ -11,7 +11,7 @@ use crate::column::ColumnIndex; use crate::error::Error; use crate::ext::ustr::UStr; use crate::row::Row; -use crate::sqlite::statement::StatementHandle; +use crate::sqlite::statement::{StatementHandle, StatementHandleRef}; use crate::sqlite::{Sqlite, SqliteColumn, SqliteValue, SqliteValueRef}; /// Implementation of [`Row`] for SQLite. @@ -23,7 +23,7 @@ pub struct SqliteRow { // IF the user drops the Row before iterating the stream (so // nearly all of our internal stream iterators), the executor moves on; otherwise, // it actually inflates this row with a list of owned sqlite3 values. - pub(crate) statement: Arc, + pub(crate) statement: StatementHandleRef, pub(crate) values: Arc>, pub(crate) num_values: usize, @@ -48,7 +48,7 @@ impl SqliteRow { // returns a weak reference to an atomic list where the executor should inflate if its going // to increment the statement with [step] pub(crate) fn current( - statement: &Arc, + statement: StatementHandleRef, columns: &Arc>, column_names: &Arc>, ) -> (Self, Weak>) { @@ -57,7 +57,7 @@ impl SqliteRow { let size = statement.column_count(); let row = Self { - statement: Arc::clone(statement), + statement, values, num_values: size, columns: Arc::clone(columns), diff --git a/sqlx-core/src/sqlite/statement/handle.rs b/sqlx-core/src/sqlite/statement/handle.rs index 78f2ff2abc..27e7b59020 100644 --- a/sqlx-core/src/sqlite/statement/handle.rs +++ b/sqlx-core/src/sqlite/statement/handle.rs @@ -20,12 +20,25 @@ use libsqlite3_sys::{ }; use crate::error::{BoxDynError, Error}; +use crate::sqlite::connection::ConnectionHandleRef; use crate::sqlite::type_info::DataType; use crate::sqlite::{SqliteError, SqliteTypeInfo}; +use std::ops::Deref; +use std::sync::Arc; #[derive(Debug)] pub(crate) struct StatementHandle(NonNull); +// wrapper for `Arc` which also holds a reference to the `ConnectionHandle` +#[derive(Clone, Debug)] +pub(crate) struct StatementHandleRef { + // NOTE: the ordering of fields here determines the drop order: + // https://doc.rust-lang.org/reference/destructors.html#destructors + // the statement *must* be dropped before the connection + statement: Arc, + connection: ConnectionHandleRef, +} + // access to SQLite3 statement handles are safe to send and share between threads // as long as the `sqlite3_step` call is serialized. @@ -292,7 +305,18 @@ impl StatementHandle { pub(crate) fn clear_bindings(&self) { unsafe { sqlite3_clear_bindings(self.0.as_ptr()) }; } + + pub(crate) fn to_ref( + self: &Arc, + conn: ConnectionHandleRef, + ) -> StatementHandleRef { + StatementHandleRef { + statement: Arc::clone(self), + connection: conn, + } + } } + impl Drop for StatementHandle { fn drop(&mut self) { // SAFETY: we have exclusive access to the `StatementHandle` here @@ -311,3 +335,11 @@ impl Drop for StatementHandle { } } } + +impl Deref for StatementHandleRef { + type Target = StatementHandle; + + fn deref(&self) -> &Self::Target { + &self.statement + } +} diff --git a/sqlx-core/src/sqlite/statement/mod.rs b/sqlx-core/src/sqlite/statement/mod.rs index dec11dcc17..97ca9f8685 100644 --- a/sqlx-core/src/sqlite/statement/mod.rs +++ b/sqlx-core/src/sqlite/statement/mod.rs @@ -12,7 +12,7 @@ mod handle; mod r#virtual; mod worker; -pub(crate) use handle::StatementHandle; +pub(crate) use handle::{StatementHandle, StatementHandleRef}; pub(crate) use r#virtual::VirtualStatement; pub(crate) use worker::StatementWorker;