diff --git a/GRDB.xcodeproj/project.pbxproj b/GRDB.xcodeproj/project.pbxproj index c0d3f02a70..f9888c4541 100755 --- a/GRDB.xcodeproj/project.pbxproj +++ b/GRDB.xcodeproj/project.pbxproj @@ -292,6 +292,7 @@ 56BB6EA91D3009B100A1CA52 /* SchedulingWatchdog.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56BB6EA81D3009B100A1CA52 /* SchedulingWatchdog.swift */; }; 56BCA2622E6C28F800E4F08D /* DatabaseSchemaSourceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56BCA2612E6C28EF00E4F08D /* DatabaseSchemaSourceTests.swift */; }; 56BF2282241781C5003D86EB /* UtilsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56BF2281241781C5003D86EB /* UtilsTests.swift */; }; + 56C7979B2EE99BD000AA265D /* Issue1838Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56C7979A2EE99BD000AA265D /* Issue1838Tests.swift */; }; 56CC922C201DFFB900CB597E /* DropWhileCursorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56CC922B201DFFB900CB597E /* DropWhileCursorTests.swift */; }; 56CC9243201E034D00CB597E /* PrefixWhileCursorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56CC9242201E034D00CB597E /* PrefixWhileCursorTests.swift */; }; 56CC9246201E058100CB597E /* DropFirstCursorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56CC9245201E058000CB597E /* DropFirstCursorTests.swift */; }; @@ -802,6 +803,7 @@ 56C3F7521CF9F12400F6A361 /* DatabaseSavepointTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabaseSavepointTests.swift; sourceTree = ""; }; 56C48E731C9A9923005DF1D9 /* module.modulemap */ = {isa = PBXFileReference; lastKnownFileType = "sourcecode.module-map"; path = module.modulemap; sourceTree = ""; }; 56C494401ED7255500CC72AF /* GRDBDeploymentTarget.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = GRDBDeploymentTarget.xcconfig; sourceTree = ""; }; + 56C7979A2EE99BD000AA265D /* Issue1838Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Issue1838Tests.swift; sourceTree = ""; }; 56CC922B201DFFB900CB597E /* DropWhileCursorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DropWhileCursorTests.swift; sourceTree = ""; }; 56CC9242201E034D00CB597E /* PrefixWhileCursorTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrefixWhileCursorTests.swift; sourceTree = ""; }; 56CC9245201E058000CB597E /* DropFirstCursorTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DropFirstCursorTests.swift; sourceTree = ""; }; @@ -1448,6 +1450,7 @@ 56B964C21DA521450002DA19 /* FTS5TableBuilderTests.swift */, 5698ACCA1DA62A2D0056AF8C /* FTS5TokenizerTests.swift */, 56ED8A7E1DAB8D6800BD0ABC /* FTS5WrapperTokenizerTests.swift */, + 56C7979A2EE99BD000AA265D /* Issue1838Tests.swift */, ); name = FTS; sourceTree = ""; @@ -2130,6 +2133,7 @@ 56D496C11D81373A008276D7 /* DatabaseQueueBackupTests.swift in Sources */, 562393721DEE104400A6B01F /* MapCursorTests.swift in Sources */, 56D496571D81303E008276D7 /* FoundationDateComponentsTests.swift in Sources */, + 56C7979B2EE99BD000AA265D /* Issue1838Tests.swift in Sources */, 568C3F7A2A5AB2C300A2309D /* ForeignKeyDefinitionTests.swift in Sources */, 56D496701D81309E008276D7 /* RecordPrimaryKeyRowIDTests.swift in Sources */, 5622060C1E420EB3005860AC /* DatabaseQueueConcurrencyTests.swift in Sources */, diff --git a/GRDB/Core/Database+Statements.swift b/GRDB/Core/Database+Statements.swift index 34a362b9dc..02229266c2 100644 --- a/GRDB/Core/Database+Statements.swift +++ b/GRDB/Core/Database+Statements.swift @@ -513,6 +513,27 @@ extension Database { publicStatementArguments: configuration.publicStatementArguments) } + /// Always throws an error + func statementCompilationDidFail( + at statementStart: UnsafePointer, + withResultCode resultCode: CInt + ) throws -> Never { + switch ResultCode(rawValue: resultCode) { + case .SQLITE_INTERRUPT, .SQLITE_ABORT: + // The only error that a user sees when a Task is cancelled + // is CancellationError. + try suspensionMutex.load().checkCancellation() + default: + break + } + + // Throw compilation failure + throw DatabaseError( + resultCode: resultCode, + message: lastErrorMessage, + sql: String(cString: statementStart)) + } + private func checkForAutocommitTransition() { if sqlite3_get_autocommit(sqliteConnection) == 0 { if autocommitState == .on { diff --git a/GRDB/Core/Database.swift b/GRDB/Core/Database.swift index 2614602ddd..d280aa7f19 100644 --- a/GRDB/Core/Database.swift +++ b/GRDB/Core/Database.swift @@ -863,7 +863,16 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib readOnlyDepth -= 1 assert(readOnlyDepth >= 0, "unbalanced endReadOnly()") if readOnlyDepth == 0 { - try internalCachedStatement(sql: "PRAGMA query_only = 0").execute() + // We MUST ignore interruptions when we leave the read-only mode, + // otherwise user could not write with this database + // connection again. + // + // It's OK to ignore interruption, since interruption is + // concurrent and we can pretend it occurred before or after + // the PRAGMA. + try ignoringInterruption { + try internalCachedStatement(sql: "PRAGMA query_only = 0").execute() + } } } @@ -1300,6 +1309,31 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib return try value() } + /// Returns the result of `value`. If `value` throws an interruption + /// error, it is retried a second time. + func ignoringInterruption(_ value: () throws -> T) rethrows -> T { + do { + return try value() + } + catch is CancellationError, + DatabaseError.SQLITE_INTERRUPT, + DatabaseError.SQLITE_ABORT + { + // Maybe we were unlucky, and user has interrupted the database + // during `value` execution. + // + // Another possible cause for this error is the FTS5 bug + // described at , + // which leaves the database in a sticky interrupted state. + // To workaround this bug, we must leave the interrupted state + // before retrying: + resetAllPreparedStatements() + + // Retry + return try value() + } + } + /// Support for `checkForSuspensionViolation(from:)` private func journalMode() throws -> String { if let journalMode = journalModeCache { @@ -1764,7 +1798,16 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib // which rollback errors should be ignored, and which rollback errors // should be exposed to the library user. if isInsideTransaction { - try execute(sql: "ROLLBACK TRANSACTION") + // We MUST ignore interruptions during the rollback, otherwise + // we could leave a transaction open, and trigger a fatal error in + // `SerializedDatabase.preconditionNoUnsafeTransactionLeft`. + // + // It's OK to ignore interruption, since interruption is + // concurrent and we can pretend it occurred before or after + // the rollback. + try ignoringInterruption { + try execute(sql: "ROLLBACK TRANSACTION") + } } assert(!isInsideTransaction) } @@ -1779,6 +1822,19 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib assert(sqlite3_get_autocommit(sqliteConnection) != 0) } + /// Resets all prepared statements. + /// + /// This method helps clearing the interrupted state, as a workaround + /// for https://sqlite.org/forum/forumpost/137c7662b3, discovered + /// in https://github.com/groue/GRDB.swift/issues/1838. + func resetAllPreparedStatements() { + var stmt: SQLiteStatement? = sqlite3_next_stmt(sqliteConnection, nil) + while stmt != nil { + sqlite3_reset(stmt) + stmt = sqlite3_next_stmt(sqliteConnection, stmt) + } + } + // MARK: - Memory Management /// Frees as much memory as possible. diff --git a/GRDB/Core/Statement.swift b/GRDB/Core/Statement.swift index 6bf3a89ad4..e47907d97d 100644 --- a/GRDB/Core/Statement.swift +++ b/GRDB/Core/Statement.swift @@ -150,10 +150,7 @@ public final class Statement { &sqliteStatement, statementEnd) guard code == SQLITE_OK else { - throw DatabaseError( - resultCode: code, - message: database.lastErrorMessage, - sql: String(cString: statementStart)) + try database.statementCompilationDidFail(at: statementStart, withResultCode: code) } guard let sqliteStatement else { diff --git a/GRDBCustom.xcodeproj/project.pbxproj b/GRDBCustom.xcodeproj/project.pbxproj index 5ae342ef80..f7c29af3ac 100755 --- a/GRDBCustom.xcodeproj/project.pbxproj +++ b/GRDBCustom.xcodeproj/project.pbxproj @@ -287,6 +287,7 @@ 56C0539722ACEECD0029D27D /* ValueReducer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56C0538C22ACEECD0029D27D /* ValueReducer.swift */; }; 56C0539B22ACEECD0029D27D /* Map.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56C0538E22ACEECD0029D27D /* Map.swift */; }; 56C0539D22ACEECD0029D27D /* RemoveDuplicates.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56C0538F22ACEECD0029D27D /* RemoveDuplicates.swift */; }; + 56C7979D2EE99BE900AA265D /* Issue1838Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56C7979C2EE99BE900AA265D /* Issue1838Tests.swift */; }; 56C7A6AE1D2DFF6100EFB0C2 /* FoundationNSDateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56F0B98E1B6001C600A2F135 /* FoundationNSDateTests.swift */; }; 56C9EB9B2495324700FE5D55 /* grdb_config.h in Headers */ = {isa = PBXBuildFile; fileRef = 56C9EB9A2495324700FE5D55 /* grdb_config.h */; settings = {ATTRIBUTES = (Private, ); }; }; 56CC9234201E009000CB597E /* DropWhileCursorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56CC9231201DFFF600CB597E /* DropWhileCursorTests.swift */; }; @@ -814,6 +815,7 @@ 56C0538F22ACEECD0029D27D /* RemoveDuplicates.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RemoveDuplicates.swift; sourceTree = ""; }; 56C3F7521CF9F12400F6A361 /* DatabaseSavepointTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabaseSavepointTests.swift; sourceTree = ""; }; 56C494421ED726C500CC72AF /* GRDBDeploymentTarget.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; name = GRDBDeploymentTarget.xcconfig; path = SQLiteCustom/GRDBDeploymentTarget.xcconfig; sourceTree = ""; }; + 56C7979C2EE99BE900AA265D /* Issue1838Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Issue1838Tests.swift; sourceTree = ""; }; 56C9EB9A2495324700FE5D55 /* grdb_config.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = grdb_config.h; path = SQLiteCustom/grdb_config.h; sourceTree = ""; }; 56CC9231201DFFF600CB597E /* DropWhileCursorTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DropWhileCursorTests.swift; sourceTree = ""; }; 56CC923A201E033400CB597E /* PrefixWhileCursorTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrefixWhileCursorTests.swift; sourceTree = ""; }; @@ -1441,6 +1443,7 @@ 5698AC3E1DA2BEBB0056AF8C /* FTS */ = { isa = PBXGroup; children = ( + 56C7979C2EE99BE900AA265D /* Issue1838Tests.swift */, 5698AC3F1DA2BED90056AF8C /* FTS3PatternTests.swift */, 5698AC481DA2D48A0056AF8C /* FTS3RecordTests.swift */, 5698AC881DA389380056AF8C /* FTS3TableBuilderTests.swift */, @@ -2374,6 +2377,7 @@ 562205FE1E420EA2005860AC /* DatabasePoolConcurrencyTests.swift in Sources */, 565EFAF11D0436CE00A8FA9D /* NumericOverflowTests.swift in Sources */, 56DF0017228DDB8300D611F3 /* AssociationPrefetchingRowTests.swift in Sources */, + 56C7979D2EE99BE900AA265D /* Issue1838Tests.swift in Sources */, F3BA812E1CFB3064003DC1BA /* RecordPrimaryKeyMultipleTests.swift in Sources */, F3BA81021CFB3032003DC1BA /* CGFloatTests.swift in Sources */, 5676FBAA22F5CEB9004717D9 /* ValueObservationRegionRecordingTests.swift in Sources */, diff --git a/Tests/GRDBTests/DatabaseWriterTests.swift b/Tests/GRDBTests/DatabaseWriterTests.swift index 9a8cf40624..2e527cdabd 100644 --- a/Tests/GRDBTests/DatabaseWriterTests.swift +++ b/Tests/GRDBTests/DatabaseWriterTests.swift @@ -896,4 +896,41 @@ class DatabaseWriterTests : GRDBTestCase { try await test(makeDatabasePool()) try await test(AnyDatabaseWriter(makeDatabaseQueue())) } + + // Regression test for https://github.com/groue/GRDB.swift/pull/1838 + func test_cancellation_does_not_prevent_rollback() async throws { + func test(_ dbWriter: some DatabaseWriter) async throws { + // Numbers that have the test fail more or less reliably + // (on my machine) unless the #1838 patch is applied. + // When the test fails, we have a fatal error: A transaction has been left opened at the end of a database access. + let repeatCount = 400 + let taskCount = 50 + for _ in 0... + /// + /// This test passes since + /// which workarounds the SQLite bug described at + /// . + func test_interrupted_rollback() throws { + let dbQueue = try makeDatabaseQueue() + + try dbQueue.inDatabase { db in + try db.execute(sql: "CREATE VIRTUAL TABLE documents USING FTS5(content)") + try db.inTransaction { + try db.execute(sql: "INSERT INTO documents(content) VALUES ('Document')") + dbQueue.interrupt() + return .rollback + } + } + + let documentCount = try dbQueue.read { db in + try Int.fetchOne(db, sql: "SELECT COUNT(*) FROM documents")! + } + XCTAssertEqual(documentCount, 0) + } + + /// Tests about how we handle the FTS5 bug + func test_interrupted_commit() throws { + let dbQueue = try makeDatabaseQueue() + + try dbQueue.write { db in + try db.execute(sql: "CREATE VIRTUAL TABLE documents USING FTS5(content)") + } + + do { + try dbQueue.write { db in + try db.execute(sql: "INSERT INTO documents(content) VALUES ('Document')") + dbQueue.interrupt() + } + // Do not assert that the above code throws, because the + // FTS5 bug might be fixed in the tested SQLite version. + } catch DatabaseError.SQLITE_INTERRUPT { + // Fine: That's the FTS5 bug. + } + + // Make sure the interrupted state created by the FTS5 bug is no longer active. + let documentCount = try dbQueue.read { db in + try Int.fetchOne(db, sql: "SELECT COUNT(*) FROM documents")! + } + XCTAssertEqual(documentCount, 0) + } +}