diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 830e15de..e809dfac 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -149,6 +149,15 @@ jobs: dart run.dart --sanitizer=msan working-directory: native_tests/ + # Regression test for https://github.com/simolus3/sqlite3.dart/issues/396 + - name: Test sqlite3_connection_pool with MemoryDenyWriteExecute + if: runner.os == 'Linux' + working-directory: sqlite3_connection_pool + run: | + dart build cli --target test/pool_test.dart -o out + systemd-run --user --pipe --wait --collect --property=MemoryDenyWriteExecute=yes --property=RuntimeMaxSec=10 ./out/bundle/bin/pool_test + rm -rf out + - name: Enable sqlite3mc run: | dart run tool/hook_overrides.dart compiled-ciphers diff --git a/sqlite3_connection_pool/lib/src/ffi.g.dart b/sqlite3_connection_pool/lib/src/ffi.g.dart index d3eed9f3..c844533c 100644 --- a/sqlite3_connection_pool/lib/src/ffi.g.dart +++ b/sqlite3_connection_pool/lib/src/ffi.g.dart @@ -29,6 +29,22 @@ external void pkg_sqlite3_connection_pool_close( ffi.Pointer pool, ); +@ffi.Native)>() +external void pkg_sqlite3_connection_pool_close_uninitialized( + ffi.Pointer uninitialized, +); + +@ffi.Native< + ffi.Pointer Function( + ffi.Pointer, + ffi.Pointer, + ) +>() +external ffi.Pointer pkg_sqlite3_connection_pool_initialize( + ffi.Pointer uninitialized, + ffi.Pointer pool, +); + @ffi.Native)>() external void pkg_sqlite3_connection_pool_notify_updates( ffi.Pointer request, @@ -65,39 +81,29 @@ external ffi.Pointer pkg_sqlite3_connection_pool_obtain_exclusive( ffi.Pointer, ffi.Int64, ffi.Int64, + ffi.Char, ) >(isLeaf: true) -external ffi.Pointer pkg_sqlite3_connection_pool_obtain_read( +external ffi.Pointer pkg_sqlite3_connection_pool_obtain_single( ffi.Pointer pool, int tag, int port, + int read, ); @ffi.Native< - ffi.Pointer Function( - ffi.Pointer, - ffi.Int64, - ffi.Int64, - ) ->(isLeaf: true) -external ffi.Pointer pkg_sqlite3_connection_pool_obtain_write( - ffi.Pointer pool, - int tag, - int port, -); - -@ffi.Native< - ffi.Pointer Function( + ffi.Void Function( ffi.Pointer, ffi.UintPtr, - ffi.Pointer Function()>>, + ffi.Pointer>, + ffi.Pointer>, ) >() -external ffi.Pointer pkg_sqlite3_connection_pool_open( +external void pkg_sqlite3_connection_pool_open( ffi.Pointer name, int name_len, - ffi.Pointer Function()>> - initialize, + ffi.Pointer> initializer, + ffi.Pointer> pool, ); @ffi.Native< @@ -174,6 +180,12 @@ class _SymbolAddresses { > get pkg_sqlite3_connection_pool_close => ffi.Native.addressOf(self.pkg_sqlite3_connection_pool_close); + ffi.Pointer< + ffi.NativeFunction)> + > + get pkg_sqlite3_connection_pool_close_uninitialized => ffi.Native.addressOf( + self.pkg_sqlite3_connection_pool_close_uninitialized, + ); ffi.Pointer)>> get pkg_sqlite3_connection_pool_request_close => ffi.Native.addressOf(self.pkg_sqlite3_connection_pool_request_close); @@ -320,3 +332,5 @@ final class PoolConnection extends ffi.Struct { } final class PoolRequest extends ffi.Opaque {} + +final class UninitializedPool extends ffi.Opaque {} diff --git a/sqlite3_connection_pool/lib/src/pool.dart b/sqlite3_connection_pool/lib/src/pool.dart index d2780ce2..808a0b45 100644 --- a/sqlite3_connection_pool/lib/src/pool.dart +++ b/sqlite3_connection_pool/lib/src/pool.dart @@ -164,7 +164,7 @@ final class SqliteConnectionPool { Future? abortSignal, ) async { _checkNotClosed(); - final (request, future) = writer ? _raw.requestWrite() : _raw.requestRead(); + final (request, future) = _raw.requestSingleConnection(!writer); _installAbortSignal(request, abortSignal); final connectionPointer = await future; diff --git a/sqlite3_connection_pool/lib/src/raw.dart b/sqlite3_connection_pool/lib/src/raw.dart index 41b648e5..1e28f6dd 100644 --- a/sqlite3_connection_pool/lib/src/raw.dart +++ b/sqlite3_connection_pool/lib/src/raw.dart @@ -93,26 +93,19 @@ final class RawSqliteConnectionPool implements Finalizable { return (id, _outstandingRequests[id] = Completer()); } - (RawPoolRequest, Future) requestRead() { - final (tag, completer) = _createRequest(); - final request = RawPoolRequest._( - tag, - this, - pkg_sqlite3_connection_pool_obtain_read(_pool, tag, _nativePort), - ); - - return ( - request, - completer.future.then((f) => (f as _SingleConnectionLease)._connection), - ); - } - - (RawPoolRequest, Future) requestWrite() { + (RawPoolRequest, Future) requestSingleConnection( + bool read, + ) { final (tag, completer) = _createRequest(); final request = RawPoolRequest._( tag, this, - pkg_sqlite3_connection_pool_obtain_write(_pool, tag, _nativePort), + pkg_sqlite3_connection_pool_obtain_single( + _pool, + tag, + _nativePort, + read ? 1 : 0, + ), ); return ( @@ -181,80 +174,75 @@ final class RawSqliteConnectionPool implements Finalizable { String name, PoolConnections Function() open, ) { - (Object, StackTrace)? openException; - - final pool = using((alloc) { + return using((alloc) { final encoded = utf8.encode(name); final namePtr = alloc(encoded.length); + final initializerAndPool = alloc>(2); + final initializerOut = initializerAndPool + .cast>(); + final poolClientOut = (initializerAndPool + 1) + .cast>(); + namePtr.asTypedList(encoded.length).setAll(0, encoded); - final initializeCallable = - NativeCallable Function()>.isolateLocal(() { - final initOptionsPtr = alloc(); - final initOptions = initOptionsPtr.ref; - initOptions.functions - ..sqlite3_update_hook = libsqlite3.addresses.sqlite3_update_hook - .cast() - ..sqlite3_rollback_hook = libsqlite3 - .addresses - .sqlite3_rollback_hook - .cast() - ..sqlite3_commit_hook = libsqlite3.addresses.sqlite3_commit_hook - .cast() - ..sqlite3_get_autocommit = libsqlite3 - .addresses - .sqlite3_get_autocommit - .cast() - ..sqlite3_finalize = libsqlite3.addresses.sqlite3_finalize.cast() - ..sqlite3_close_v2 = libsqlite3.addresses.sqlite3_close_v2.cast() - ..dart_post_c_object = NativeApi.postCObject.cast(); - - try { - final PoolConnections( - :readers, - :writer, - :preparedStatementCacheSize, - :enableNativeUpdateHooks, - ) = open(); - - initOptions.write = writer.leak().cast(); - initOptions.read_count = readers.length; - initOptions.reads = alloc(readers.length); - initOptions.prepared_statement_cache_size = - preparedStatementCacheSize; - initOptions.enable_update_hooks = enableNativeUpdateHooks ? 1 : 0; - - for (final (i, reader) in readers.indexed) { - (initOptions.reads + i).value = reader.leak().cast(); - } - } catch (e, s) { - openException = (e, s); - return nullptr; - } - - return initOptionsPtr; - }); - - final connection = pkg_sqlite3_connection_pool_open( + pkg_sqlite3_connection_pool_open( namePtr, encoded.length, - initializeCallable.nativeFunction, + initializerOut, + poolClientOut, ); - initializeCallable.close(); - return connection; - }); - if (pool.address == 0) { - if (openException case (final exception, final trace)?) { - // Couldn't open because the callback threw an exception, rethrow that. - Error.throwWithStackTrace(exception, trace); - } + final initializer = initializerOut.value; + final poolClient = poolClientOut.value; - // Unreachable, opening a pool can only fail due to the callback throwing. - throw AssertionError(); - } + // If a pool with this name already exists, it is written to poolClient. + if (poolClient.address != 0) { + return RawSqliteConnectionPool._(poolClient); + } - return RawSqliteConnectionPool._(pool); + // Otherwise, we're given an initializer and it's our responsibility to + // open the pool now. + assert(initializer.address != 0); + + final initOptionsPtr = alloc(); + final initOptions = initOptionsPtr.ref; + initOptions.functions + ..sqlite3_update_hook = libsqlite3.addresses.sqlite3_update_hook.cast() + ..sqlite3_rollback_hook = libsqlite3.addresses.sqlite3_rollback_hook + .cast() + ..sqlite3_commit_hook = libsqlite3.addresses.sqlite3_commit_hook.cast() + ..sqlite3_get_autocommit = libsqlite3.addresses.sqlite3_get_autocommit + .cast() + ..sqlite3_finalize = libsqlite3.addresses.sqlite3_finalize.cast() + ..sqlite3_close_v2 = libsqlite3.addresses.sqlite3_close_v2.cast() + ..dart_post_c_object = NativeApi.postCObject.cast(); + + try { + final PoolConnections( + :readers, + :writer, + :preparedStatementCacheSize, + :enableNativeUpdateHooks, + ) = open(); + + initOptions.write = writer.leak().cast(); + initOptions.read_count = readers.length; + initOptions.reads = alloc(readers.length); + initOptions.prepared_statement_cache_size = preparedStatementCacheSize; + initOptions.enable_update_hooks = enableNativeUpdateHooks ? 1 : 0; + + for (final (i, reader) in readers.indexed) { + (initOptions.reads + i).value = reader.leak().cast(); + } + + return RawSqliteConnectionPool._( + pkg_sqlite3_connection_pool_initialize(initializer, initOptionsPtr), + ); + } on Object { + pkg_sqlite3_connection_pool_close_uninitialized(initializer); + rethrow; + } + }); } } diff --git a/sqlite3_connection_pool/src/headers.h b/sqlite3_connection_pool/src/headers.h index dabc2fda..66468da8 100644 --- a/sqlite3_connection_pool/src/headers.h +++ b/sqlite3_connection_pool/src/headers.h @@ -3,6 +3,7 @@ typedef struct ConnectionPool ConnectionPool; typedef struct PoolRequest PoolRequest; +typedef struct UninitializedPool UninitializedPool; typedef const void* Connection; @@ -29,22 +30,21 @@ typedef struct InitializedPool { unsigned char enable_update_hooks; } InitializedPool; -typedef struct InitializedPool* (*PoolInitializer)(void); - typedef int64_t DartPort; -ConnectionPool* pkg_sqlite3_connection_pool_open(const uint8_t* name, - uintptr_t name_len, - PoolInitializer initialize); +void pkg_sqlite3_connection_pool_open(const uint8_t* name, uintptr_t name_len, + UninitializedPool** initializer, + ConnectionPool** pool); -void pkg_sqlite3_connection_pool_close(const ConnectionPool* pool); +ConnectionPool* pkg_sqlite3_connection_pool_initialize( + UninitializedPool* uninitialized, const InitializedPool* pool); +void pkg_sqlite3_connection_pool_close_uninitialized( + UninitializedPool* uninitialized); -PoolRequest* pkg_sqlite3_connection_pool_obtain_read(const ConnectionPool* pool, - int64_t tag, - DartPort port); +void pkg_sqlite3_connection_pool_close(const ConnectionPool* pool); -PoolRequest* pkg_sqlite3_connection_pool_obtain_write( - const ConnectionPool* pool, int64_t tag, DartPort port); +PoolRequest* pkg_sqlite3_connection_pool_obtain_single( + const ConnectionPool* pool, int64_t tag, DartPort port, char read); PoolRequest* pkg_sqlite3_connection_pool_obtain_exclusive( const ConnectionPool* pool, int64_t tag, DartPort port); diff --git a/sqlite3_connection_pool/src/lib.rs b/sqlite3_connection_pool/src/lib.rs index 2de019cc..6ce31c15 100644 --- a/sqlite3_connection_pool/src/lib.rs +++ b/sqlite3_connection_pool/src/lib.rs @@ -2,9 +2,10 @@ use crate::client::PoolClient; use crate::connection::{Connection, PreparedStatement}; use crate::dart::DartPort; use crate::pool::{ConnectionPool, PendingMessage, PoolConnection, PoolRequestHandle, PoolState}; -use crate::registry::{PoolInitializer, PoolRegistry}; +use crate::registry::{InitializedPool, MaybeInitializedPool, PoolRegistry, UninitializedPool}; use crate::update_hook::send_update_notification; use std::ffi::{CStr, c_char, c_int, c_void}; +use std::mem::MaybeUninit; use std::ptr::NonNull; use std::sync::{Arc, Mutex}; use std::{ptr, slice}; @@ -16,19 +17,61 @@ mod pool; mod registry; mod update_hook; +fn to_client(pool: ConnectionPool) -> NonNull { + let boxed = Box::new(PoolClient::new(pool)); + let ptr = Box::into_raw(boxed); + unsafe { NonNull::new_unchecked(ptr) } +} + +/// Attempts to open a connection pool with the given utf-8 encoded name. +/// +/// If a pool with that name exists, it is written to `client` and `initializer` is set to [None]. +/// +/// Otherwise `client` is set to [None] and the [UninitializedPool] is heap-allocated and written to +/// `initializer`. The unitialized pool must be completed with either a call to +/// [pkg_sqlite3_connection_pool_initialize] or to +/// [pkg_sqlite3_connection_pool_close_uninitialized]. #[unsafe(no_mangle)] extern "C" fn pkg_sqlite3_connection_pool_open( name: *const u8, name_len: usize, - initialize: PoolInitializer, -) -> Option> { + initializer: &mut MaybeUninit>>>, + client: &mut MaybeUninit>>, +) { let name = unsafe { str::from_utf8_unchecked(slice::from_raw_parts(name, name_len)) }; - PoolRegistry::lookup(name, initialize).map(|pool| { - let client = PoolClient::new(pool); + match PoolRegistry::lookup(name) { + MaybeInitializedPool::Pool(pool) => { + initializer.write(None); + client.write(Some(to_client(pool))); + } + MaybeInitializedPool::Uninitialized(uninitialized_pool) => { + client.write(None); + let boxed = Box::new(uninitialized_pool); + let ptr = Box::into_raw(boxed); + initializer.write(Some(unsafe { NonNull::new_unchecked(ptr) })); + } + } +} + +/// Consumes an uninitialized pool by transforming it into an opened connection pool. +#[unsafe(no_mangle)] +extern "C" fn pkg_sqlite3_connection_pool_initialize( + uninitialized: NonNull, + initialize: &InitializedPool, +) -> NonNull { + let guard = unsafe { Box::from_raw(uninitialized.as_ptr()) }; + to_client(guard.initialize(initialize)) +} - unsafe { NonNull::new_unchecked(Box::into_raw(Box::new(client))) } - }) +/// Consumes an uninitialized pool by releasing resources without turning it into an opened +/// connection pool. +#[unsafe(no_mangle)] +extern "C" fn pkg_sqlite3_connection_pool_close_uninitialized( + uninitialized: NonNull, +) { + let guard = unsafe { Box::from_raw(uninitialized.as_ptr()) }; + drop(guard); } #[unsafe(no_mangle)] @@ -45,33 +88,21 @@ fn clone_arc(pool: &Mutex) -> ConnectionPool { } #[unsafe(no_mangle)] -extern "C" fn pkg_sqlite3_connection_pool_obtain_read( +extern "C" fn pkg_sqlite3_connection_pool_obtain_single( client: &PoolClient, tag: i64, port: DartPort, + read: c_char, ) -> *mut PoolRequestHandle { let pool = &client.pool; let mut state = pool.lock().unwrap(); let pool = clone_arc(pool); - Box::into_raw(Box::new( - state.request_read(pool, PendingMessage { tag, port }), - )) -} - -#[unsafe(no_mangle)] -extern "C" fn pkg_sqlite3_connection_pool_obtain_write( - client: &PoolClient, - tag: i64, - port: DartPort, -) -> *mut PoolRequestHandle { - let pool = &client.pool; - let mut state = pool.lock().unwrap(); - let pool = clone_arc(pool); - - Box::into_raw(Box::new( - state.request_write(pool, PendingMessage { tag, port }), - )) + Box::into_raw(Box::new(state.request_single( + pool, + PendingMessage { tag, port }, + read != 0, + ))) } #[unsafe(no_mangle)] diff --git a/sqlite3_connection_pool/src/pool.rs b/sqlite3_connection_pool/src/pool.rs index f81e525b..94c33f85 100644 --- a/sqlite3_connection_pool/src/pool.rs +++ b/sqlite3_connection_pool/src/pool.rs @@ -240,16 +240,21 @@ impl PoolState { } } - pub fn request_read(&mut self, pool: ConnectionPool, msg: PendingMessage) -> PoolRequestHandle { - self.register_waiter(pool, msg, Waiter::Reader(Default::default())) - } - - pub fn request_write( + pub fn request_single( &mut self, pool: ConnectionPool, msg: PendingMessage, + read: bool, ) -> PoolRequestHandle { - self.register_waiter(pool, msg, Waiter::Writer(Default::default())) + self.register_waiter( + pool, + msg, + if read { + Waiter::Reader(Default::default()) + } else { + Waiter::Writer(Default::default()) + }, + ) } pub fn request_exclusive( diff --git a/sqlite3_connection_pool/src/registry.rs b/sqlite3_connection_pool/src/registry.rs index acccd846..3157a605 100644 --- a/sqlite3_connection_pool/src/registry.rs +++ b/sqlite3_connection_pool/src/registry.rs @@ -2,9 +2,8 @@ use crate::connection::Connection; use crate::pool::{ConnectionPool, ExternalFunctions, PoolState}; use std::collections::HashMap; use std::ffi::c_uchar; -use std::ptr::NonNull; use std::slice; -use std::sync::{Arc, LazyLock, Mutex, Weak}; +use std::sync::{Arc, LazyLock, Mutex, MutexGuard, Weak}; static REGISTRY: LazyLock = LazyLock::new(|| PoolRegistry::default()); @@ -13,6 +12,16 @@ pub struct PoolRegistry { pools: Mutex>>>, } +pub struct UninitializedPool<'a> { + name: &'a str, + guard: MutexGuard<'a, HashMap>>>, +} + +pub enum MaybeInitializedPool<'a> { + Pool(ConnectionPool), + Uninitialized(UninitializedPool<'a>), +} + #[repr(C)] pub struct InitializedPool { functions: ExternalFunctions, @@ -23,27 +32,25 @@ pub struct InitializedPool { enable_update_hooks: c_uchar, } -pub type PoolInitializer = extern "C" fn() -> Option>; - impl PoolRegistry { - fn lookup_internal(&self, name: &str, initialize: PoolInitializer) -> Option { - let mut pools = self.pools.lock().unwrap(); + fn lookup_internal<'a>(&'a self, name: &'a str) -> MaybeInitializedPool<'a> { + let pools = self.pools.lock().unwrap(); if let Some(pool) = pools.get(name) { if let Some(pool) = Weak::upgrade(pool) { - return Some(pool); + return MaybeInitializedPool::Pool(pool); } }; - // The pool doesn't exist, obtain connections from Dart callback. - let Some(initialized) = initialize() else { - // Initialization failed, don't insert a pool. - return None; - }; - let initialized = unsafe { - // The returned pointer is valid until this function returns. - initialized.as_ref() - }; + return MaybeInitializedPool::Uninitialized(UninitializedPool { name, guard: pools }); + } + + pub fn lookup<'a>(name: &'a str) -> MaybeInitializedPool<'a> { + REGISTRY.lookup_internal(name) + } +} +impl<'a> UninitializedPool<'a> { + pub fn initialize(mut self, initialized: &InitializedPool) -> ConnectionPool { let state = PoolState::new( initialized.functions, initialized.write, @@ -55,11 +62,8 @@ impl PoolRegistry { let pool = ConnectionPool::new(Mutex::new(state)); PoolState::register_hooks_on_writer(&pool); - pools.insert(name.to_string(), Arc::downgrade(&pool)); - Some(pool) - } - - pub fn lookup(name: &str, initialize: PoolInitializer) -> Option { - REGISTRY.lookup_internal(name, initialize) + self.guard + .insert(self.name.to_string(), Arc::downgrade(&pool)); + pool } }