Skip to content

Commit f704845

Browse files
committed
Use RwLock instead of RefCell for local connection
The `local::Connection` struct has the following safety comments: ```rust // SAFETY: This is safe because we compile sqlite3 w/ SQLITE_THREADSAFE=1 unsafe impl Send for Connection {} // SAFETY: This is safe because we compile sqlite3 w/ SQLITE_THREADSAFE=1 unsafe impl Sync for Connection {} ``` This is correct with respect to the `*mut ffi::sqlite3` field, but NOT with respect to the `RefCell<Option<AuthHook>>` field. Heavy load in a real application using local databases occasionally sees panics due to concurrent access to this RefCell.
1 parent a2e9dd4 commit f704845

File tree

2 files changed

+15
-14
lines changed

2 files changed

+15
-14
lines changed

libsql/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ core = [
6363
"dep:bitflags",
6464
"dep:bytes",
6565
"dep:futures",
66+
"dep:parking_lot",
6667
]
6768
stream = [
6869
"dep:futures",

libsql/src/local/connection.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ use super::{Database, Error, Result, Rows, RowsFuture, Statement, Transaction};
1212
use crate::TransactionBehavior;
1313

1414
use libsql_sys::ffi;
15-
use std::cell::RefCell;
1615
use std::{ffi::c_int, fmt, path::Path, sync::Arc};
16+
use parking_lot::RwLock;
1717

1818
/// A connection to a libSQL database.
1919
#[derive(Clone)]
@@ -25,7 +25,7 @@ pub struct Connection {
2525
#[cfg(feature = "replication")]
2626
pub(crate) writer: Option<crate::replication::Writer>,
2727

28-
authorizer: RefCell<Option<AuthHook>>,
28+
authorizer: Arc<RwLock<Option<AuthHook>>>,
2929
}
3030

3131
impl Drop for Connection {
@@ -68,7 +68,7 @@ impl Connection {
6868
drop_ref: Arc::new(()),
6969
#[cfg(feature = "replication")]
7070
writer: db.writer()?,
71-
authorizer: RefCell::new(None),
71+
authorizer: Arc::new(RwLock::new(None)),
7272
};
7373
#[cfg(feature = "sync")]
7474
if let Some(_) = db.sync_ctx {
@@ -95,7 +95,7 @@ impl Connection {
9595
drop_ref: Arc::new(()),
9696
#[cfg(feature = "replication")]
9797
writer: None,
98-
authorizer: RefCell::new(None),
98+
authorizer: Arc::new(RwLock::new(None)),
9999
}
100100
}
101101

@@ -475,7 +475,7 @@ impl Connection {
475475
}
476476
}
477477

478-
*self.authorizer.borrow_mut() = hook.clone();
478+
*self.authorizer.write() = hook.clone();
479479

480480
let (callback, user_data) = match hook {
481481
Some(_) => {
@@ -604,7 +604,7 @@ impl Connection {
604604

605605
pub(crate) fn wal_insert_handle(&self) -> Result<WalInsertHandle<'_>> {
606606
self.wal_insert_begin()?;
607-
Ok(WalInsertHandle { conn: self, in_session: RefCell::new(true) })
607+
Ok(WalInsertHandle { conn: self, in_session: RwLock::new(true) })
608608
}
609609
}
610610

@@ -617,7 +617,7 @@ unsafe extern "C" fn authorizer_callback(
617617
accessor: *const ::std::os::raw::c_char,
618618
) -> ::std::os::raw::c_int {
619619
let conn = user_data as *const Connection;
620-
let hook = unsafe { (*conn).authorizer.borrow() };
620+
let hook = unsafe { (*conn).authorizer.read() };
621621
let hook = match &*hook {
622622
Some(hook) => hook,
623623
None => return ffi::SQLITE_OK,
@@ -658,33 +658,33 @@ unsafe extern "C" fn authorizer_callback(
658658

659659
pub(crate) struct WalInsertHandle<'a> {
660660
conn: &'a Connection,
661-
in_session: RefCell<bool>
661+
in_session: RwLock<bool>
662662
}
663663

664664
impl WalInsertHandle<'_> {
665665
pub fn insert(&self, frame: &[u8]) -> Result<()> {
666-
assert!(*self.in_session.borrow());
666+
assert!(*self.in_session.read());
667667
self.conn.wal_insert_frame(frame)
668668
}
669669

670670
pub fn begin(&self) -> Result<()> {
671-
assert!(!*self.in_session.borrow());
671+
assert!(!*self.in_session.read());
672672
self.conn.wal_insert_begin()?;
673-
self.in_session.replace(true);
673+
*self.in_session.write() = true;
674674
Ok(())
675675
}
676676

677677
pub fn end(&self) -> Result<()> {
678-
assert!(*self.in_session.borrow());
678+
assert!(*self.in_session.read());
679679
self.conn.wal_insert_end()?;
680-
self.in_session.replace(false);
680+
*self.in_session.write() = false;
681681
Ok(())
682682
}
683683
}
684684

685685
impl Drop for WalInsertHandle<'_> {
686686
fn drop(&mut self) {
687-
if *self.in_session.borrow() {
687+
if *self.in_session.read() {
688688
if let Err(err) = self.conn.wal_insert_end() {
689689
tracing::error!("{:?}", err);
690690
Err(err).unwrap()

0 commit comments

Comments
 (0)