Skip to content

Commit b455dff

Browse files
committed
Don't use BEGIN IMMEDIATE on the web
1 parent d0e493c commit b455dff

File tree

7 files changed

+53
-62
lines changed

7 files changed

+53
-62
lines changed

packages/sqlite_async/lib/src/common/connection/sync_sqlite_connection.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:sqlite_async/src/update_notification.dart';
88
import 'package:sqlite_async/src/utils/profiler.dart';
99

1010
import '../../impl/context.dart';
11+
import '../../utils/shared_utils.dart';
1112

1213
/// A simple "synchronous" connection which provides the async SqliteConnection
1314
/// implementation using a synchronous SQLite connection
@@ -35,6 +36,16 @@ final class SyncSqliteConnection extends SqliteConnection {
3536
);
3637
}
3738

39+
@override
40+
Future<T> readTransaction<T>(
41+
Future<T> Function(SqliteReadContext tx) callback,
42+
{Duration? lockTimeout}) {
43+
return readLock((ctx) async {
44+
return await internalReadTransaction(ctx, callback,
45+
isDedicatedReadConnection: false);
46+
}, lockTimeout: lockTimeout, debugContext: 'readTransaction()');
47+
}
48+
3849
@override
3950
Future<T> abortableReadLock<T>(
4051
Future<T> Function(SqliteReadContext tx) callback,

packages/sqlite_async/lib/src/common/sqlite_database.dart

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import 'package:sqlite_async/src/sqlite_options.dart';
77
import 'package:sqlite_async/src/sqlite_connection.dart';
88

99
import '../impl/platform.dart' as platform;
10+
import '../utils/shared_utils.dart';
1011

1112
/// A SQLite database instance.
1213
///
@@ -96,4 +97,14 @@ abstract base class SqliteDatabase extends SqliteConnection {
9697
@internal
9798
abstract base class SqliteDatabaseImpl extends SqliteDatabase {
9899
SqliteDatabaseImpl() : super._();
100+
101+
@override
102+
Future<T> readTransaction<T>(
103+
Future<T> Function(SqliteReadContext tx) callback,
104+
{Duration? lockTimeout}) {
105+
return readLock((ctx) async {
106+
return await internalReadTransaction(ctx, callback,
107+
isDedicatedReadConnection: maxReaders > 0);
108+
}, lockTimeout: lockTimeout, debugContext: 'readTransaction()');
109+
}
99110
}

packages/sqlite_async/lib/src/native/database/native_sqlite_database.dart

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -94,58 +94,6 @@ final class NativeSqliteDatabaseImpl extends SqliteDatabaseImpl {
9494
}
9595
}
9696

97-
/// Open a read-only transaction.
98-
///
99-
/// Up to [maxReaders] read transactions can run concurrently.
100-
/// After that, read transactions are queued.
101-
///
102-
/// Read transactions can run concurrently to a write transaction.
103-
///
104-
/// Changes from any write transaction are not visible to read transactions
105-
/// started before it.
106-
@override
107-
Future<T> readTransaction<T>(
108-
Future<T> Function(SqliteReadContext tx) callback,
109-
{Duration? lockTimeout}) async {
110-
return _useConnection(
111-
writer: false,
112-
abortTrigger: lockTimeout?.asTimeout,
113-
debugContext: 'readTransaction',
114-
(context) {
115-
return _transactionInLease(context, callback);
116-
},
117-
);
118-
}
119-
120-
/// Open a read-write transaction.
121-
///
122-
/// Only a single write transaction can run at a time - any concurrent
123-
/// transactions are queued.
124-
///
125-
/// The write transaction is automatically committed when the callback finishes,
126-
/// or rolled back on any error.
127-
@override
128-
Future<T> writeTransaction<T>(
129-
Future<T> Function(SqliteWriteContext tx) callback,
130-
{Duration? lockTimeout}) {
131-
return _useConnection(
132-
writer: true,
133-
abortTrigger: lockTimeout?.asTimeout,
134-
debugContext: 'writeTransaction',
135-
(context) {
136-
return _transactionInLease(context, callback);
137-
},
138-
);
139-
}
140-
141-
Future<T> _transactionInLease<T>(
142-
_LeasedContext context,
143-
Future<T> Function(SqliteWriteContext tx) callback,
144-
) {
145-
final ctx = ScopedWriteContext(context);
146-
return ctx.writeTransaction(callback).whenComplete(ctx.invalidate);
147-
}
148-
14997
@override
15098
Future<T> abortableReadLock<T>(
15199
Future<T> Function(SqliteReadContext tx) callback,

packages/sqlite_async/lib/src/sqlite_connection.dart

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,7 @@ abstract class SqliteConnection implements SqliteWriteContext {
136136
/// instance will error.
137137
Future<T> readTransaction<T>(
138138
Future<T> Function(SqliteReadContext tx) callback,
139-
{Duration? lockTimeout}) {
140-
return readLock((ctx) async {
141-
return await internalReadTransaction(ctx, callback);
142-
}, lockTimeout: lockTimeout, debugContext: 'readTransaction()');
143-
}
139+
{Duration? lockTimeout});
144140

145141
/// Open a read-write transaction.
146142
///

packages/sqlite_async/lib/src/utils/shared_utils.dart

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,35 @@ import 'package:sqlite3/common.dart';
55

66
import '../sqlite_connection.dart';
77

8-
Future<T> internalReadTransaction<T>(SqliteReadContext ctx,
9-
Future<T> Function(SqliteReadContext tx) callback) async {
8+
Future<T> internalReadTransaction<T>(
9+
SqliteReadContext ctx,
10+
Future<T> Function(SqliteReadContext tx) callback, {
11+
required bool isDedicatedReadConnection,
12+
}) async {
1013
try {
11-
await ctx.getAll('BEGIN IMMEDIATE');
14+
// We want read transactions to observe the state of the database at the
15+
// time they've been opened. By default however, SQLite only starts the
16+
// transaction on the first statement (BEGIN just sets a bit to disable
17+
// autocommit). This is a problem for snippets like:
18+
//
19+
// await db.readTransaction((tx) async {
20+
// // point in time 1.
21+
// await longDelay();
22+
// await readFromDb(tx); // should read state from point in time 1!
23+
// });
24+
//
25+
// With a write concurrent to `longDelay()`, the actual transaction would
26+
// start too late and observe state from after it was opened in Dart. This
27+
// is why we use BEGIN IMMEDIATE instead of BEGIN. However, we only need
28+
// this for read connections: If the database "pool" is backed by a single
29+
// connection (e.g. on the web), using BEGIN IMMEDIATE would be fairly
30+
// expensive for a read. A concurrent write wouldn't be a concern there
31+
// because the read context blocks the single connection.
32+
// Either way, this is not a consistency issue. Transactions observe a
33+
// single consistent snapshot either way, we just want them to observe an
34+
// earlier snapshot in some cases.
35+
36+
await ctx.getAll(isDedicatedReadConnection ? 'BEGIN IMMEDIATE' : 'BEGIN');
1237
final result = await callback(ctx);
1338
await ctx.getAll('END TRANSACTION');
1439
return result;

packages/sqlite_async/lib/src/web/database.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ final class WebDatabase extends SqliteDatabaseImpl
6868
@override
6969

7070
/// Not supported on web. There is only 1 connection.
71-
int get maxReaders => throw UnimplementedError();
71+
int get maxReaders => 0;
7272

7373
@override
7474

packages/sqlite_async/lib/src/web/database/async_web_database.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ final class AsyncWebDatabaseImpl extends SqliteDatabaseImpl
2929
late Stream<UpdateNotification> updates;
3030

3131
@override
32-
int get maxReaders => openFactory.sqliteOptions.maxReaders;
32+
int get maxReaders => 0;
3333

3434
@override
3535
@protected

0 commit comments

Comments
 (0)