Avoid callbacks when opening connection pools

.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

sqlite3_connection_pool/lib/src/ffi.g.dart

@@ -29,6 +29,22 @@ external void pkg_sqlite3_connection_pool_close(
   ffi.Pointer<ConnectionPool> pool,
 );
 
+@ffi.Native<ffi.Void Function(ffi.Pointer<UninitializedPool>)>()
+external void pkg_sqlite3_connection_pool_close_uninitialized(
+  ffi.Pointer<UninitializedPool> uninitialized,
+);
+
+@ffi.Native<
+  ffi.Pointer<ConnectionPool> Function(
+    ffi.Pointer<UninitializedPool>,
+    ffi.Pointer<InitializedPool>,
+  )
+>()
+external ffi.Pointer<ConnectionPool> pkg_sqlite3_connection_pool_initialize(
+  ffi.Pointer<UninitializedPool> uninitialized,
+  ffi.Pointer<InitializedPool> pool,
+);
+
 @ffi.Native<ffi.Void Function(ffi.Pointer<PoolRequest>)>()
 external void pkg_sqlite3_connection_pool_notify_updates(
   ffi.Pointer<PoolRequest> request,
@@ -65,39 +81,29 @@ external ffi.Pointer<PoolRequest> pkg_sqlite3_connection_pool_obtain_exclusive(
     ffi.Pointer<ConnectionPool>,
     ffi.Int64,
     ffi.Int64,
+    ffi.Char,
   )
 >(isLeaf: true)
-external ffi.Pointer<PoolRequest> pkg_sqlite3_connection_pool_obtain_read(
+external ffi.Pointer<PoolRequest> pkg_sqlite3_connection_pool_obtain_single(
   ffi.Pointer<ConnectionPool> pool,
   int tag,
   int port,
+  int read,
 );
 
 @ffi.Native<
-  ffi.Pointer<PoolRequest> Function(
-    ffi.Pointer<ConnectionPool>,
-    ffi.Int64,
-    ffi.Int64,
-  )
->(isLeaf: true)
-external ffi.Pointer<PoolRequest> pkg_sqlite3_connection_pool_obtain_write(
-  ffi.Pointer<ConnectionPool> pool,
-  int tag,
-  int port,
-);
-
-@ffi.Native<
-  ffi.Pointer<ConnectionPool> Function(
+  ffi.Void Function(
     ffi.Pointer<ffi.Uint8>,
     ffi.UintPtr,
-    ffi.Pointer<ffi.NativeFunction<ffi.Pointer<InitializedPool> Function()>>,
+    ffi.Pointer<ffi.Pointer<UninitializedPool>>,
+    ffi.Pointer<ffi.Pointer<ConnectionPool>>,
   )
 >()
-external ffi.Pointer<ConnectionPool> pkg_sqlite3_connection_pool_open(
+external void pkg_sqlite3_connection_pool_open(
   ffi.Pointer<ffi.Uint8> name,
   int name_len,
-  ffi.Pointer<ffi.NativeFunction<ffi.Pointer<InitializedPool> Function()>>
-  initialize,
+  ffi.Pointer<ffi.Pointer<UninitializedPool>> initializer,
+  ffi.Pointer<ffi.Pointer<ConnectionPool>> 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<ffi.Void Function(ffi.Pointer<UninitializedPool>)>
+  >
+  get pkg_sqlite3_connection_pool_close_uninitialized => ffi.Native.addressOf(
+    self.pkg_sqlite3_connection_pool_close_uninitialized,
+  );
   ffi.Pointer<ffi.NativeFunction<ffi.Void Function(ffi.Pointer<PoolRequest>)>>
   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 {}

sqlite3_connection_pool/lib/src/pool.dart

@@ -164,7 +164,7 @@ final class SqliteConnectionPool {
     Future<void>? abortSignal,
   ) async {
     _checkNotClosed();
-    final (request, future) = writer ? _raw.requestWrite() : _raw.requestRead();
+    final (request, future) = _raw.requestSingleConnection(!writer);
     _installAbortSignal(request, abortSignal);
 
     final connectionPointer = await future;

sqlite3_connection_pool/lib/src/raw.dart

@@ -93,26 +93,19 @@ final class RawSqliteConnectionPool implements Finalizable {
     return (id, _outstandingRequests[id] = Completer());
   }
 
-  (RawPoolRequest, Future<PoolConnectionRef>) 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<PoolConnectionRef>) requestWrite() {
+  (RawPoolRequest, Future<PoolConnectionRef>) 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<Uint8>(encoded.length);
+      final initializerAndPool = alloc<Pointer<Void>>(2);
+      final initializerOut = initializerAndPool
+          .cast<Pointer<UninitializedPool>>();
+      final poolClientOut = (initializerAndPool + 1)
+          .cast<Pointer<ConnectionPool>>();
+
       namePtr.asTypedList(encoded.length).setAll(0, encoded);
 
-      final initializeCallable =
-          NativeCallable<Pointer<InitializedPool> Function()>.isolateLocal(() {
-            final initOptionsPtr = alloc<InitializedPool>();
-            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<InitializedPool>();
+      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;
+      }
+    });
   }
 }
 

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);