Skip to content

Commit

Permalink
Merge pull request #1716 from groue/dev/issue-1715
Browse files Browse the repository at this point in the history
Fix: DatabaseQueue restores its read/write abilities when an async read-only database access is cancelled.
  • Loading branch information
groue authored Feb 4, 2025
2 parents 287c4be + 5a4d9b5 commit 3535a42
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 0 deletions.
6 changes: 6 additions & 0 deletions GRDB/Core/Database.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1288,6 +1288,12 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
return
}

// Suspension should not prevent adjusting the read-only mode.
// See <https://github.com/groue/GRDB.swift/issues/1715>.
if statement.isQueryOnlyPragma {
return
}

// How should we interrupt the statement?
enum Interrupt {
case abort // Rollback and throw SQLITE_ABORT
Expand Down
4 changes: 4 additions & 0 deletions GRDB/Core/Statement.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ public final class Statement {
/// The effects on the database (reported by `sqlite3_set_authorizer`).
private(set) var authorizerEventKinds: [DatabaseEventKind] = []

/// If true, the statement executes is a `PRAGMA QUERY_ONLY` statement.
private(set) var isQueryOnlyPragma = false

/// A boolean value indicating if the prepared statement makes no direct
/// changes to the content of the database file.
///
Expand Down Expand Up @@ -160,6 +163,7 @@ public final class Statement {
self.invalidatesDatabaseSchemaCache = authorizer.invalidatesDatabaseSchemaCache
self.transactionEffect = authorizer.transactionEffect
self.authorizerEventKinds = authorizer.databaseEventKinds
self.isQueryOnlyPragma = authorizer.isQueryOnlyPragma
}

deinit {
Expand Down
9 changes: 9 additions & 0 deletions GRDB/Core/StatementAuthorizer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ final class StatementAuthorizer {
/// savepoint statement.
var transactionEffect: Statement.TransactionEffect?

/// If true, the statement executes is a `PRAGMA QUERY_ONLY` statement.
var isQueryOnlyPragma = false

private var isDropStatement = false

init(_ database: Database) {
Expand All @@ -67,6 +70,7 @@ final class StatementAuthorizer {
databaseEventKinds = []
invalidatesDatabaseSchemaCache = false
transactionEffect = nil
isQueryOnlyPragma = false
isDropStatement = false
}

Expand Down Expand Up @@ -192,6 +196,11 @@ final class StatementAuthorizer {
}
return SQLITE_OK

case SQLITE_PRAGMA:
if let cString1 {
isQueryOnlyPragma = sqlite3_stricmp(cString1, "query_only") == 0
}
return SQLITE_OK
default:
return SQLITE_OK
}
Expand Down
51 changes: 51 additions & 0 deletions Tests/GRDBTests/DatabaseWriterTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,57 @@ class DatabaseWriterTests : GRDBTestCase {

// MARK: - Task Cancellation

// Regression test for <https://github.com/groue/GRDB.swift/issues/1715>.
func test_write_is_possible_after_read_cancelled_after_database_access() async throws {
// When a read access is cancelled, DatabaseQueue needs to execute
// `PRAGMA query_only=0` in order to restore the read/write access.
//
// Here we test that this pragma can run from a cancelled read.
//
// Small difficulty: some SQLite versions (seen with 3.43.2) execute
// the `query_only` pragma at compile time, not only at execution
// time (yeah, that's an SQLite bug). The problem of this bug is
// that even if the `PRAGMA query_only=0` is not executed due to
// Task cancellation, its side effect is still executed when it is
// compiled, unintentionally. A cancelled `PRAGMA query_only=0`
// still works!
//
// To avoid this SQLite bug from messing with our test, we perform
// two reads: one that compiles and cache `PRAGMA query_only`
// statements, and a second read that we cancel. This time the
// `PRAGMA query_only=0` triggers its side effect if and only if it
// is actually executed (the behavior we are testing).
func test(_ dbWriter: some DatabaseWriter) async throws {
let semaphore = AsyncSemaphore(value: 0)
let cancelledTaskMutex = Mutex<Task<Void, any Error>?>(nil)
let task = Task {
await semaphore.wait()

// First read, not cancelled, so that all `query_only`
// pragma statements are compiled (see above).
try await dbWriter.read { db in }

// Second read, cancelled.
try await dbWriter.read { db in
try XCTUnwrap(cancelledTaskMutex.load()).cancel()
}
}
cancelledTaskMutex.store(task)
semaphore.signal()
// Wait until reads are completed
try? await task.value

// Write access is restored after read cancellation (no error is thrown)
try await dbWriter.write { db in
try db.execute(sql: "CREATE TABLE test(a)")
}
}

try await test(makeDatabaseQueue())
try await test(makeDatabasePool())
try await test(AnyDatabaseWriter(makeDatabaseQueue()))
}

func test_writeWithoutTransaction_is_cancelled_by_Task_cancellation_performed_before_database_access() async throws {
func test(_ dbWriter: some DatabaseWriter) async throws {
let semaphore = AsyncSemaphore(value: 0)
Expand Down

0 comments on commit 3535a42

Please sign in to comment.