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

Removes sqlite3_config and sqlite3_initialize from local db new #1887

Closed
Closed
Show file tree
Hide file tree
Changes from all 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
16 changes: 15 additions & 1 deletion libsql/src/local/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,28 @@ impl Connection {
pub(crate) fn connect(db: &Database) -> Result<Connection> {
let mut raw = std::ptr::null_mut();
let db_path = db.db_path.clone();
let flags: c_int = ffi::SQLITE_OPEN_FULLMUTEX | db.flags.bits();

let err = unsafe {
if ffi::sqlite3_threadsafe() == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is actually safe, because accroding to the docs a user can still enable the threading model in such a way that it invalidates the assumptions the rust code has via sqlite_confgi

The return value of the sqlite3_threadsafe() function shows only the compile-time setting of thread safety, not any run-time changes to that setting made by sqlite3_config().

So this to me isn't strong enough to enforce 100% (esp with sqlite having global config) that the send/sync bounds we have in rust are safely implemented.

// https://sqlite.org/threadsafe.html
return Err(Error::ConnectionFailed(String::from(
"SQLite thread safety check failed: SQLite was compiled without thread-safety support \
(SQLITE_THREADSAFE=0). For libsql to operate correctly, SQLite must be compiled with either \
serialized mode (SQLITE_THREADSAFE=1) or multi-threaded mode (SQLITE_THREADSAFE=2). \
Thread-safety is required because libsql needs to ensure safe concurrent access to database \
connections across multiple threads. \
Please rebuild SQLite with the appropriate thread-safety options."
)));
}

ffi::sqlite3_open_v2(
std::ffi::CString::new(db_path.as_str())
.unwrap()
.as_c_str()
.as_ptr() as *const _,
&mut raw,
db.flags.bits() as c_int,
flags,
std::ptr::null(),
)
};
Expand Down
38 changes: 10 additions & 28 deletions libsql/src/local/database.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::sync::Once;

cfg_replication!(
use http::uri::InvalidUri;
use crate::database::{EncryptionConfig, FrameNo};
Expand All @@ -24,7 +22,6 @@ cfg_sync! {

use crate::{database::OpenFlags, local::connection::Connection};
use crate::{Error::ConnectionFailed, Result};
use libsql_sys::ffi;

// A libSQL database.
pub struct Database {
Expand Down Expand Up @@ -230,29 +227,6 @@ impl Database {
}

pub fn new(db_path: String, flags: OpenFlags) -> Database {
static LIBSQL_INIT: Once = Once::new();

LIBSQL_INIT.call_once(|| {
// Ensure that we are configured with the correct threading model
// if this config is not set correctly the entire api is unsafe.
unsafe {
assert_eq!(
ffi::sqlite3_config(ffi::SQLITE_CONFIG_SERIALIZED),
ffi::SQLITE_OK,
"libsql was configured with an incorrect threading configuration and
the api is not safe to use. Please check that no multi-thread options have
been set. If nothing was configured then please open an issue at:
https://github.com/libsql/libsql"
);

assert_eq!(
ffi::sqlite3_initialize(),
ffi::SQLITE_OK,
"libsql failed to initialize"
);
}
});

Database {
db_path,
flags,
Expand Down Expand Up @@ -420,7 +394,11 @@ impl Database {
}

#[cfg(feature = "sync")]
async fn try_push(&self, sync_ctx: &mut SyncContext, conn: &Connection) -> Result<crate::database::Replicated> {
async fn try_push(
&self,
sync_ctx: &mut SyncContext,
conn: &Connection,
) -> Result<crate::database::Replicated> {
let page_size = {
let rows = conn
.query("PRAGMA page_size", crate::params::Params::None)?
Expand Down Expand Up @@ -471,7 +449,11 @@ impl Database {
}

#[cfg(feature = "sync")]
async fn try_pull(&self, sync_ctx: &mut SyncContext, conn: &Connection) -> Result<crate::database::Replicated> {
async fn try_pull(
&self,
sync_ctx: &mut SyncContext,
conn: &Connection,
) -> Result<crate::database::Replicated> {
let generation = sync_ctx.generation();
let mut frame_no = sync_ctx.durable_frame_num() + 1;
conn.wal_insert_begin()?;
Expand Down
Loading