diff --git a/drift/CHANGELOG.md b/drift/CHANGELOG.md index d26092227..5e16bc27b 100644 --- a/drift/CHANGELOG.md +++ b/drift/CHANGELOG.md @@ -1,3 +1,7 @@ +## Unreleased + +- Web: Fix nested transactions failing to commit and hanging on rollback with `WasmStorageImplementation.opfsLocks`. A nested transaction completed the shared navigator-lock completer, so the outer commit failed with `Bad state: Future already completed` and its rollback never resolved (#3870). + ## 2.35.1 - Web: Properly release transactions held by closing tabs. diff --git a/drift/lib/src/web/wasm_setup/navigator_locks_interceptor.dart b/drift/lib/src/web/wasm_setup/navigator_locks_interceptor.dart index 1ea4cf0d1..8134d49a2 100644 --- a/drift/lib/src/web/wasm_setup/navigator_locks_interceptor.dart +++ b/drift/lib/src/web/wasm_setup/navigator_locks_interceptor.dart @@ -19,15 +19,17 @@ final class NavigatorLocksExecutor implements QueryExecutor { @override QueryExecutor beginExclusive() { - return _inner.beginExclusive().interceptWith( - _AcquireNavigatorLockInterceptor(this), + final inner = _inner.beginExclusive(); + return inner.interceptWith( + _AcquireNavigatorLockInterceptor(this, ownsLock: inner), ); } @override TransactionExecutor beginTransaction() { - return _inner.beginTransaction().interceptWith( - _AcquireNavigatorLockInterceptor(this), + final inner = _inner.beginTransaction(); + return inner.interceptWith( + _AcquireNavigatorLockInterceptor(this, ownsLock: inner), ) as TransactionExecutor; } @@ -103,9 +105,18 @@ final class _AcquireNavigatorLockInterceptor extends QueryInterceptor { final NavigatorLocksExecutor _executor; final Completer _returnNavigatorLocks = Completer(); + /// The executor whose commit, rollback or close ends this interceptor's + /// navigator-lock scope. Executors for nested transactions flow through + /// this same interceptor but must not complete the shared completer. + /// See https://github.com/simolus3/drift/issues/3870 + final QueryExecutor _ownsNavigatorLock; + Future? _acquiredNavigatorLock; - _AcquireNavigatorLockInterceptor(this._executor); + _AcquireNavigatorLockInterceptor( + this._executor, { + required QueryExecutor ownsLock, + }) : _ownsNavigatorLock = ownsLock; @override Future ensureOpen( @@ -122,18 +133,25 @@ final class _AcquireNavigatorLockInterceptor extends QueryInterceptor { @override Future close(QueryExecutor inner) { - return inner.close().whenComplete(() => _returnNavigatorLocks.complete()); + return inner.close().whenComplete(() => _releaseLock(inner)); } @override Future commitTransaction(TransactionExecutor inner) { - return inner.send().whenComplete(() => _returnNavigatorLocks.complete()); + return inner.send().whenComplete(() => _releaseLock(inner)); } @override Future rollbackTransaction(TransactionExecutor inner) { - return inner.rollback().whenComplete( - () => _returnNavigatorLocks.complete(), - ); + return inner.rollback().whenComplete(() => _releaseLock(inner)); + } + + void _releaseLock(QueryExecutor inner) { + // A nested transaction completes before the outermost one; releasing the + // navigator lock here would make the outer commit fail with + // `Bad state: Future already completed` and its rollback hang forever. + if (identical(inner, _ownsNavigatorLock)) { + _returnNavigatorLocks.complete(); + } } } diff --git a/drift/test/platforms/web/navigator_locks_interceptor_test.dart b/drift/test/platforms/web/navigator_locks_interceptor_test.dart index 2a42c213e..8dcceaf58 100644 --- a/drift/test/platforms/web/navigator_locks_interceptor_test.dart +++ b/drift/test/platforms/web/navigator_locks_interceptor_test.dart @@ -40,6 +40,37 @@ void main() { expect(await db.todosTable.all().get(), isNotEmpty); }); + test('nested transactions complete the outer transaction', () async { + // Regression test for https://github.com/simolus3/drift/issues/3870: + // the nested transaction used to complete the shared navigator-lock + // completer, so the outer commit failed and its rollback hung forever. + final db = TodoDb(connect()); + addTearDown(db.close); + + await db.transaction(() async { + await db.transaction(() async {}); + }); + }); + + test('nested transaction with rollback rolls the outer one back', () async { + final db = TodoDb(connect()); + addTearDown(db.close); + + final outer = db.transaction(() async { + await db + .transaction(() async { + await db.todosTable.insertOne( + TodosTableCompanion.insert(content: 'nested'), + ); + }) + .then((_) => throw Exception('rollback after nested commit')); + }); + // The nested commit succeeded, but the outer transaction rolls back. + await expectLater(outer, throwsA(isA())); + + expect(await db.todosTable.all().get(), isEmpty); + }); + test("can't have concurrent transactions", () async { var concurrentTransactions = 0; var totalTransactions = 0;