Skip to content

Commit

Permalink
fix: lock issue reported tekartik/sembast_sqflite#5 and with a good b…
Browse files Browse the repository at this point in the history
…ackground of information here simolus3/drift#835

The solution from Simon Binder is simple and brilliant
  • Loading branch information
alextekartik committed Oct 2, 2020
1 parent d28248a commit fe2a925
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 48 deletions.
97 changes: 97 additions & 0 deletions sqflite_common_ffi/lib/src/database_tracker.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import 'dart:ffi';

import 'package:sqlite3/sqlite3.dart';

/// Copied from moor
/// This entire file is an elaborate hack to workaround https://github.com/simolus3/moor/issues/835.
///
/// Users were running into database deadlocks after (stateless) hot restarts
/// in Flutter when they use transactions. The problem is that we don't have a
/// chance to call `sqlite3_close` before a Dart VM restart, the Dart object is
/// just gone without a trace. This means that we're leaking sqlite3 database
/// connections on restarts.
/// Even worse, those connections might have a lock on the database, for
/// instance if they just started a transaction.
///
/// Our solution is to store open sqlite3 database connections in an in-memory
/// sqlite database which can survive restarts! For now, we keep track of the
/// pointer of an sqlite3 database handle in that database.
/// At an early stage of their `main()` method, users can now use
/// `VmDatabase.closeExistingInstances()` to release those resources.
DatabaseTracker get tracker => _tracker ??= DatabaseTracker();
DatabaseTracker _tracker;

var _tableName = 'open_connections';
var _ptrColName = 'db_pointer';

/// Internal class that we don't export to sqflite users. See [tracker] for why
/// this is necessary.
class DatabaseTracker {
/// Creates a new tracker with necessary tables.
///
/// If the table exists delete any open connection.
DatabaseTracker()
: _db = sqlite3.open(
'file:sqflite_database_tracker?mode=memory&cache=shared',
uri: true,
) {
try {
var tableExists = (_db
.select(
'SELECT COUNT(*) FROM sqlite_master WHERE tbl_name = \'$_tableName\'')
.first
.columnAt(0) as int) >
0;
if (!tableExists) {
_db.execute('''
CREATE TABLE IF NOT EXISTS $_tableName (
$_ptrColName INTEGER NOT NULL PRIMARY KEY
);
''');
} else {
_closeExisting();
}
} catch (e) {
print('error $e creating tracker db');
}
}

final Database _db;

/// Tracks the [db]. The [path] argument can be used to track the path
/// of that database, if it's bound to a file.
void markOpened(Database db) {
final ptr = db.handle.address;
try {
_db.execute('INSERT INTO $_tableName($_ptrColName) VALUES($ptr)');
} catch (_) {
// Handle when the same pointer is inserted twice
// sqlite tends to reuse the same pointer
}
}

/// Marks the database [db] as closed.
void markClosed(Database db) {
final ptr = db.handle.address;
_db.execute('DELETE FROM $_tableName WHERE $_ptrColName = $ptr');
}

/// Closes tracked database connections.
void _closeExisting() {
_db.execute('BEGIN;');
try {
final results = _db.select('SELECT $_ptrColName FROM $_tableName');
for (final row in results) {
final ptr = Pointer.fromAddress(row.columnAt(0) as int);
try {
sqlite3.fromPointer(ptr).dispose();
} catch (e) {
print('error $e disposing $ptr');
}
}
_db.execute('DELETE FROM $_tableName;');
} finally {
_db.execute('COMMIT;');
}
}
}
60 changes: 13 additions & 47 deletions sqflite_common_ffi/lib/src/sqflite_ffi_impl.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,17 @@ import 'dart:io';
import 'dart:typed_data';

import 'package:meta/meta.dart';
import 'package:sqlite3/sqlite3.dart' as ffi;
import 'package:path/path.dart';
import 'package:sqflite_common/sqlite_api.dart';
import 'package:sqflite_common_ffi/src/constant.dart';
import 'package:sqflite_common_ffi/src/method_call.dart';
import 'package:sqflite_common_ffi/src/sqflite_ffi_exception.dart';
import 'package:sqflite_common_ffi/src/sqflite_import.dart';
import 'package:sqlite3/sqlite3.dart' as ffi;
import 'package:synchronized/extension.dart';
import 'package:synchronized/synchronized.dart';

import 'database_tracker.dart';
import 'import.dart';

final _debug = false; // devWarning(true); // false
Expand Down Expand Up @@ -248,52 +249,6 @@ extension SqfliteFfiMethodCallHandler on FfiMethodCall {

var ffiException = wrapAnyException(e);
throw ffiException;
/*
if (e is ffi.SqliteException) {
var database = getDatabase();
var sql = getSql();
var sqlArguments = getSqlArguments();
var wrapped = wrapSqlException(e, details: <String, dynamic>{
'database': database.toDebugMap(),
'sql': sql,
'arguments': sqlArguments
});
// devPrint(wrapped);
throw wrapped;
}
var database = getDatabase();
var sql = getSql();
var sqlArguments = getSqlArguments();
if (_debug) {
print('$e in ${database?.toDebugMap()}');
}
String code;
String message;
Map<String, dynamic> details;
if (e is SqfliteFfiException) {
// devPrint('throwing $e');
code = e.code;
message = e.message;
details = e.details;
} else {
code = anyErrorCode;
message = e.toString();
}
if (_debug) {
print('handleError: $e');
print('stackTrace : $st');
}
throw SqfliteFfiException(
code: code,
message: message,
details: <String, dynamic>{
if (database != null) 'database': database.toDebugMap(),
if (sql != null) 'sql': sql,
if (sqlArguments != null) 'arguments': sqlArguments,
if (details != null) 'details': details,
});
*/
}
}

Expand Down Expand Up @@ -374,6 +329,12 @@ extension SqfliteFfiMethodCallHandler on FfiMethodCall {
final mode =
readOnly ? ffi.OpenMode.readOnly : ffi.OpenMode.readWriteCreate;
ffiDb = ffi.sqlite3.open(path, mode: mode);

// Handle hot-restart for single instance
// The dart code is killed but the native code remains
if (singleInstance) {
tracker.markOpened(ffiDb);
}
}
} on ffi.SqliteException catch (e) {
throw wrapSqlException(e, code: 'open_failed');
Expand All @@ -399,6 +360,11 @@ extension SqfliteFfiMethodCallHandler on FfiMethodCall {
var database = getDatabaseOrThrow();
if (database.singleInstance ?? false) {
ffiSingleInstanceDbs.remove(database.path);

// Handle hot-restart for single instance
// The dart code is killed but the native code remains
// Remove the database from our cache
tracker.markClosed(database._ffiDb);
}
database.close();
}
Expand Down
2 changes: 1 addition & 1 deletion sqflite_common_ffi/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ environment:
sdk: ">=2.8.0 <3.0.0"

dependencies:
sqlite3: '>=0.1.4 <1.0.0'
sqlite3: '>=0.1.6 <1.0.0'
sqflite_common: '>=1.0.1 <3.0.0'
synchronized: '>=2.0.2 <4.0.0'
path: '>=1.5.1 <3.0.0'
Expand Down

0 comments on commit fe2a925

Please sign in to comment.