Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: DatabaseQueue restores its read/write abilities when an async read-only database access is cancelled. #1716

Merged
merged 2 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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