Skip to content

Commit 67b34dc

Browse files
committed
TransactionObserver can access the database, even if the transaction was completed in a cancelled task.
Fix groue#1746
1 parent 515bd78 commit 67b34dc

File tree

3 files changed

+50
-12
lines changed

3 files changed

+50
-12
lines changed

GRDB/Core/Database+Statements.swift

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -494,11 +494,9 @@ extension Database {
494494

495495
switch ResultCode(rawValue: resultCode) {
496496
case .SQLITE_INTERRUPT, .SQLITE_ABORT:
497-
if suspensionMutex.load().isCancelled {
498-
// The only error that a user sees when a Task is cancelled
499-
// is CancellationError.
500-
throw CancellationError()
501-
}
497+
// The only error that a user sees when a Task is cancelled
498+
// is CancellationError.
499+
try suspensionMutex.load().checkCancellation()
502500
default:
503501
break
504502
}

GRDB/Core/Database.swift

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -341,10 +341,22 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
341341

342342
/// If true, the database access has been cancelled.
343343
var isCancelled: Bool
344+
345+
/// If true, the database throws an error when it is cancelled.
346+
var interruptsWhenCancelled: Bool
347+
348+
func checkCancellation() throws {
349+
if isCancelled, interruptsWhenCancelled {
350+
throw CancellationError()
351+
}
352+
}
344353
}
345354

346355
/// Support for `checkForSuspensionViolation(from:)`
347-
let suspensionMutex = Mutex(Suspension(isSuspended: false, isCancelled: false))
356+
let suspensionMutex = Mutex(Suspension(
357+
isSuspended: false,
358+
isCancelled: false,
359+
interruptsWhenCancelled: true))
348360

349361
// MARK: - Transaction Date
350362

@@ -1222,7 +1234,7 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
12221234
}
12231235

12241236
suspension.isCancelled = true
1225-
return true
1237+
return suspension.interruptsWhenCancelled
12261238
}
12271239

12281240
if needsInterrupt {
@@ -1237,6 +1249,24 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
12371249
}
12381250
}
12391251

1252+
/// Within the given closure, Task cancellation does not interrupt
1253+
/// database accesses.
1254+
func ignoringCancellation<T>(_ value: () throws -> T) rethrows -> T {
1255+
let previous = suspensionMutex.withLock {
1256+
let previous = $0.interruptsWhenCancelled
1257+
$0.interruptsWhenCancelled = false
1258+
return previous
1259+
}
1260+
1261+
defer {
1262+
suspensionMutex.withLock {
1263+
$0.interruptsWhenCancelled = previous
1264+
}
1265+
}
1266+
1267+
return try value()
1268+
}
1269+
12401270
/// Support for `checkForSuspensionViolation(from:)`
12411271
private func journalMode() throws -> String {
12421272
if let journalMode = journalModeCache {
@@ -1303,7 +1333,7 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
13031333
let interrupt: Interrupt? = try suspensionMutex.withLock { suspension in
13041334
// Check for cancellation first, so that the only error that
13051335
// a user sees when a Task is cancelled is CancellationError.
1306-
if suspension.isCancelled {
1336+
if suspension.isCancelled, suspension.interruptsWhenCancelled {
13071337
return .cancel
13081338
}
13091339

GRDB/Core/TransactionObserver.swift

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -549,8 +549,12 @@ class DatabaseObservationBroker {
549549
savepointStack.clear()
550550

551551
if !database.isReadOnly {
552-
for observation in transactionObservations {
553-
observation.databaseDidCommit(database)
552+
// Observers must be able to access the database, even if the
553+
// task that has performed the commit is cancelled.
554+
database.ignoringCancellation {
555+
for observation in transactionObservations {
556+
observation.databaseDidCommit(database)
557+
}
554558
}
555559
}
556560

@@ -609,12 +613,18 @@ class DatabaseObservationBroker {
609613

610614
// Called from statementDidExecute or statementDidFail
611615
private func databaseDidRollback(notifyTransactionObservers: Bool) {
616+
#warning("TODO")
612617
savepointStack.clear()
613618

614619
if notifyTransactionObservers {
615620
assert(!database.isReadOnly, "Read-only transactions are not notified")
616-
for observation in transactionObservations {
617-
observation.databaseDidRollback(database)
621+
622+
// Observers must be able to access the database, even if the
623+
// task that has performed the commit is cancelled.
624+
database.ignoringCancellation {
625+
for observation in transactionObservations {
626+
observation.databaseDidRollback(database)
627+
}
618628
}
619629
}
620630
databaseDidEndTransaction()

0 commit comments

Comments
 (0)