Web: Consistently use finalizers

Author: simolus3

sqlite3/CHANGELOG.md

@@ -5,7 +5,11 @@
   - The SQLite library can only be customized with user defines.
   - You should drop your dependencies on `sqlite3_flutter_libs` and
     `sqlcipher_flutter_libs` when upgrading.
+  - You can also remove dependencies on `sqlite3_native_assets`, since that
+    package is now part of `package:sqlite3`.
 - __Breaking change__: Parameters to `SqliteException`s are now named.
+- On native platforms, use native finalizers to reliably clear statements and databases.
+- Refactor binding text and blob values to reduce the chance of memory leaks.
 
 ## 2.9.4
 

sqlite3/lib/src/ffi/bindings.dart

@@ -735,7 +735,7 @@ final class FfiChangesetIterator implements RawChangesetIterator, Finalizable {
   }
 
   @override
-  SqliteResult<RawSqliteValue> sqlite3changeset_new(int columnNumber) {
+  SqliteResult<RawSqliteValue?> sqlite3changeset_new(int columnNumber) {
     final outValue = allocate<Pointer<sqlite3_value>>();
     final result = libsqlite3.sqlite3changeset_new(
       iterator,
@@ -757,7 +757,7 @@ final class FfiChangesetIterator implements RawChangesetIterator, Finalizable {
   }
 
   @override
-  SqliteResult<RawSqliteValue> sqlite3changeset_old(int columnNumber) {
+  SqliteResult<RawSqliteValue?> sqlite3changeset_old(int columnNumber) {
     final outValue = allocate<Pointer<sqlite3_value>>();
     final result = libsqlite3.sqlite3changeset_old(
       iterator,

sqlite3/lib/src/implementation/bindings.dart

@@ -10,6 +10,11 @@ import 'package:meta/meta.dart';
 import '../functions.dart';
 import '../vfs.dart';
 
+/// A `sqlite3_changeset_iter` instance.
+///
+/// For iterators created through `sqlite3changeset_start`, implementations
+/// should use finalizers to automatically call `sqlite3changeset_finalize`
+/// even when [sqlite3changeset_finalize] is not called explicitly.
 abstract interface class RawChangesetIterator {
   // int sqlite3changeset_finalize(sqlite3_changeset_iter *pIter);
   int sqlite3changeset_finalize();
@@ -19,7 +24,7 @@ abstract interface class RawChangesetIterator {
   //   int iVal,                       /* Column number */
   //   sqlite3_value **ppValue         /* OUT: New value (or NULL pointer) */
   // );
-  SqliteResult<RawSqliteValue> sqlite3changeset_new(int columnNumber);
+  SqliteResult<RawSqliteValue?> sqlite3changeset_new(int columnNumber);
 
   // int sqlite3changeset_next(sqlite3_changeset_iter *pIter);
   int sqlite3changeset_next();
@@ -29,7 +34,7 @@ abstract interface class RawChangesetIterator {
   //   int iVal,                       /* Column number */
   //   sqlite3_value **ppValue         /* OUT: Old value (or NULL pointer) */
   // );
-  SqliteResult<RawSqliteValue> sqlite3changeset_old(int columnNumber);
+  SqliteResult<RawSqliteValue?> sqlite3changeset_old(int columnNumber);
 
   // int sqlite3changeset_op(
   //   sqlite3_changeset_iter *pIter,  /* Iterator object */
@@ -130,6 +135,11 @@ abstract interface class RawSqliteBindings {
   int sqlite3_initialize();
 }
 
+/// A `sqlite3_session` instance.
+///
+/// Implementations should use finalizers to automatically calls
+/// `sqlite3session_delete` even when [sqlite3session_delete] is not called
+/// explcitly.
 abstract interface class RawSqliteSession {
   // int sqlite3session_attach(
   //   sqlite3_session *pSession,      /* Session object */
@@ -166,7 +176,7 @@ abstract interface class RawSqliteSession {
 }
 
 /// Combines a sqlite result code and the result object.
-typedef SqliteResult<T extends Object> = ({T? result, int resultCode});
+typedef SqliteResult<T> = ({T? result, int resultCode});
 
 typedef RawXFunc = void Function(RawSqliteContext, List<RawSqliteValue>);
 typedef RawXStep = void Function(RawSqliteContext, List<RawSqliteValue>);
@@ -176,6 +186,10 @@ typedef RawCommitHook = int Function();
 typedef RawRollbackHook = void Function();
 typedef RawCollation = int Function(String? a, String? b);
 
+/// A `sqlite3` instance.
+///
+/// Instances should use finalizers to automatically close the database, even
+/// when [sqlite3_close_v2] is not called explicitly.
 abstract interface class RawSqliteDatabase {
   int sqlite3_changes();
   int sqlite3_last_insert_rowid();
@@ -252,6 +266,10 @@ abstract interface class RawStatementCompiler {
   void close();
 }
 
+/// A `sqlite3_stmt` instance.
+///
+/// Instances should use finalizers to automatically call `sqlite3_finalize` on
+/// the statement, even when [sqlite3_finalize] is not called explicitly.
 abstract interface class RawSqliteStatement {
   void sqlite3_reset();
   int sqlite3_step();

sqlite3/lib/src/implementation/session.dart

@@ -187,22 +187,16 @@ final class ChangesetIteratorImplementation implements ChangesetIterator {
       final kind = SqliteUpdateKind.fromCode(op.operation)!;
 
       final oldColumns = kind != SqliteUpdateKind.insert
-          ? List.generate(
-              op.columnCount,
-              (i) => raw
-                  .sqlite3changeset_old(i)
-                  .okOrThrowOutsideOfDatabase(bindings)
-                  .read(),
-            )
+          ? List.generate(op.columnCount, (i) {
+              final rawValue = raw.sqlite3changeset_old(i);
+              return _readChangesetValue(rawValue);
+            })
           : null;
       final newColumns = kind != SqliteUpdateKind.delete
-          ? List.generate(
-              op.columnCount,
-              (i) => raw
-                  .sqlite3changeset_new(i)
-                  .okOrThrowOutsideOfDatabase(bindings)
-                  .read(),
-            )
+          ? List.generate(op.columnCount, (i) {
+              final rawValue = raw.sqlite3changeset_new(i);
+              return _readChangesetValue(rawValue);
+            })
           : null;
       current = ChangesetOperation(
         table: op.tableName,
@@ -219,6 +213,13 @@ final class ChangesetIteratorImplementation implements ChangesetIterator {
     return false;
   }
 
+  Object? _readChangesetValue(SqliteResult<RawSqliteValue?> result) {
+    return switch (result.resultCode) {
+      0 => result.result?.read(),
+      _ => throw createExceptionOutsideOfDatabase(bindings, result.resultCode),
+    };
+  }
+
   @override
   void finalize() {
     if (_isFinalized) {

sqlite3/lib/src/implementation/utils.dart

@@ -1,6 +1,5 @@
 import '../constants.dart';
 import 'bindings.dart';
-import 'exception.dart';
 
 extension BigIntRangeCheck on BigInt {
   BigInt get checkRange {
@@ -27,16 +26,6 @@ int eTextRep(bool deterministic, bool directOnly, bool subtype) {
   return flags;
 }
 
-extension HandleResult<T extends Object> on SqliteResult<T> {
-  T okOrThrowOutsideOfDatabase(RawSqliteBindings bindings) {
-    if (resultCode != SqlError.SQLITE_OK) {
-      throw createExceptionOutsideOfDatabase(bindings, resultCode);
-    }
-
-    return result!;
-  }
-}
-
 extension ReadDartValue on RawSqliteValue {
   Object? read() {
     return switch (sqlite3_value_type()) {

sqlite3/lib/src/wasm/bindings.dart

@@ -229,11 +229,15 @@ final class WasmSqliteBindings implements RawSqliteBindings {
 final class WasmDatabase implements RawSqliteDatabase {
   final wasm.WasmBindings bindings;
   final Pointer db;
+  final Object detach = Object();
 
-  WasmDatabase(this.bindings, this.db);
+  WasmDatabase(this.bindings, this.db) {
+    bindings.databaseFinalizer?.attach(this, db, detach: detach);
+  }
 
   @override
   int sqlite3_close_v2() {
+    bindings.databaseFinalizer?.detach(detach);
     return bindings.sqlite3_close_v2(db);
   }
 
@@ -448,8 +452,11 @@ final class WasmStatement implements RawSqliteStatement {
   final WasmDatabase database;
   final Pointer stmt;
   final WasmBindings bindings;
+  final Object detach = Object();
 
-  WasmStatement(this.database, this.stmt) : bindings = database.bindings;
+  WasmStatement(this.database, this.stmt) : bindings = database.bindings {
+    bindings.statementFinalizer?.attach(this, stmt, detach: detach);
+  }
 
   @override
   int sqlite3_bind_blob64(int index, List<int> value) {
@@ -576,6 +583,7 @@ final class WasmStatement implements RawSqliteStatement {
   @override
   void sqlite3_finalize() {
     bindings.sqlite3_finalize(stmt);
+    bindings.statementFinalizer?.detach(detach);
   }
 
   @override
@@ -770,18 +778,14 @@ class WasmValueList extends ListBase<WasmValue> {
 }
 
 final class WasmSession implements RawSqliteSession {
-  static final Finalizer<(WasmBindings, int)> _finalizer = Finalizer((args) {
-    args.$1.sqlite3session_delete(args.$2);
-  });
-
   final WasmSqliteBindings bindings;
   final int pointer; // the sqlite3_session ptr
   final Object detach = Object();
 
   final WasmBindings _bindings;
 
   WasmSession(this.bindings, this.pointer) : _bindings = bindings.bindings {
-    _finalizer.attach(this, (_bindings, pointer), detach: detach);
+    _bindings.sessionFinalizer?.attach(this, pointer, detach: detach);
   }
 
   @override
@@ -828,7 +832,7 @@ final class WasmSession implements RawSqliteSession {
 
   @override
   void sqlite3session_delete() {
-    _finalizer.detach(this);
+    _bindings.sessionFinalizer?.detach(this);
     _bindings.sqlite3session_delete(pointer);
   }
 
@@ -859,16 +863,6 @@ final class WasmSession implements RawSqliteSession {
 }
 
 final class WasmChangesetIterator implements RawChangesetIterator {
-  static final Finalizer<(WasmBindings, int?, int)> _finalizer = Finalizer((
-    args,
-  ) {
-    if (args.$2 case final underlyingBytes?) {
-      args.$1.free(underlyingBytes);
-    }
-
-    args.$1.sqlite3changeset_finalize(args.$3);
-  });
-
   final WasmSqliteBindings bindings;
 
   /// If this iterator was created from an uint8list allocated when creating it,
@@ -886,28 +880,29 @@ final class WasmChangesetIterator implements RawChangesetIterator {
     bool owned = true,
   }) : _bindings = bindings.bindings {
     if (owned) {
-      _finalizer.attach(this, (
-        _bindings,
-        dataPointer,
+      bindings.bindings.changesetFinalizer?.attach(
+        this,
         pointer,
-      ), detach: detach);
+        detach: detach,
+      );
     }
   }
 
   @override
   int sqlite3changeset_finalize() {
-    _finalizer.detach(detach);
+    final rc = _bindings.sqlite3changeset_finalize(pointer);
+    bindings.bindings.changesetFinalizer?.detach(detach);
     if (dataPointer case final data?) {
       _bindings.free(data);
     }
 
-    return _bindings.sqlite3changeset_finalize(pointer);
+    return rc;
   }
 
   @override
   int sqlite3changeset_next() => _bindings.sqlite3changeset_next(pointer);
 
-  SqliteResult<RawSqliteValue> _extractValue(
+  SqliteResult<RawSqliteValue?> _extractValue(
     int Function(Pointer, int, Pointer) extract,
     int index,
   ) {
@@ -923,12 +918,12 @@ final class WasmChangesetIterator implements RawChangesetIterator {
   }
 
   @override
-  SqliteResult<RawSqliteValue> sqlite3changeset_old(int columnNumber) {
+  SqliteResult<RawSqliteValue?> sqlite3changeset_old(int columnNumber) {
     return _extractValue(_bindings.sqlite3changeset_old, columnNumber);
   }
 
   @override
-  SqliteResult<RawSqliteValue> sqlite3changeset_new(int columnNumber) {
+  SqliteResult<RawSqliteValue?> sqlite3changeset_new(int columnNumber) {
     return _extractValue(_bindings.sqlite3changeset_new, columnNumber);
   }
 

sqlite3/lib/src/wasm/wasm_interop.dart

@@ -25,11 +25,24 @@ class WasmBindings {
   final DartCallbacks callbacks;
   final SqliteExports sqlite3;
 
+  // Finalizers are deliberately referenced from this instance only - if the
+  // entire instance and module is GCed, we don't need to manually invoke
+  // finalizers anymore.
+  Finalizer<Pointer>? changesetFinalizer,
+      sessionFinalizer,
+      databaseFinalizer,
+      statementFinalizer;
+
   WasmBindings._(this.instance, _InjectedValues values)
     : memory = values.memory,
       callbacks = values.callbacks,
       sqlite3 = SqliteExports(instance.exports) {
     values.bindings = this;
+
+    changesetFinalizer = Finalizer((p) => sqlite3.sqlite3changeset_finalize(p));
+    sessionFinalizer = Finalizer((p) => sqlite3.sqlite3session_delete(p));
+    databaseFinalizer = Finalizer((p) => sqlite3.sqlite3_close_v2(p));
+    statementFinalizer = Finalizer((p) => sqlite3.sqlite3_finalize(p));
   }
 
   static Future<WasmBindings> instantiateAsync(web.Response response) async {

sqlite3/test/common/session.dart

@@ -152,7 +152,7 @@ void testSession(FutureOr<CommonSqlite3> Function() loadSqlite) {
 
     session = Session(database)..diff('another', 'entries');
     final changeset = session.changeset();
-    expect(changeset, [
+    expect(changeset.toList(), [
       isOp(
         operation: SqliteUpdateKind.update,
         oldValues: [1, 'b'],