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

GRDB 7: Move closer to SE-0433 Mutex #1604

Merged
merged 1 commit into from
Aug 25, 2024
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
12 changes: 8 additions & 4 deletions GRDB.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
563C67B324628BEA00E94EDC /* DatabasePoolTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 563C67B224628BEA00E94EDC /* DatabasePoolTests.swift */; };
563CBBE12A595131008905CE /* SQLIndexGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 563CBBE02A595131008905CE /* SQLIndexGenerator.swift */; };
563DE4F3231A91E2005081B7 /* DatabaseConfigurationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 563DE4EC231A91E2005081B7 /* DatabaseConfigurationTests.swift */; };
563EA3E12C7B3A22001BE0D4 /* Mutex.swift in Sources */ = {isa = PBXBuildFile; fileRef = 563EA3E02C7B3A22001BE0D4 /* Mutex.swift */; };
563EF415215F87EB007DAACD /* OrderedDictionary.swift in Sources */ = {isa = PBXBuildFile; fileRef = 563EF414215F87EB007DAACD /* OrderedDictionary.swift */; };
563EF42D2161180D007DAACD /* AssociationAggregate.swift in Sources */ = {isa = PBXBuildFile; fileRef = 563EF42C2161180D007DAACD /* AssociationAggregate.swift */; };
563EF43F216131D1007DAACD /* AssociationAggregateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 563EF43E216131D1007DAACD /* AssociationAggregateTests.swift */; };
Expand Down Expand Up @@ -186,7 +187,7 @@
566B912B1FA4D0CC0012D5B0 /* StatementAuthorizer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 566B912A1FA4D0CC0012D5B0 /* StatementAuthorizer.swift */; };
566B91331FA4D3810012D5B0 /* TransactionObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 566B91321FA4D3810012D5B0 /* TransactionObserver.swift */; };
566B9C2025C6CC24004542CF /* RowDecodingError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 566B9C1F25C6CC24004542CF /* RowDecodingError.swift */; };
566BE71E2342542F00A8254B /* LockedBox.swift in Sources */ = {isa = PBXBuildFile; fileRef = 566BE7172342542F00A8254B /* LockedBox.swift */; };
566BE71E2342542F00A8254B /* Mutex.swift in Sources */ = {isa = PBXBuildFile; fileRef = 566BE7172342542F00A8254B /* Mutex.swift */; };
566DDE0D288D763C0000DCFB /* Fixits.swift in Sources */ = {isa = PBXBuildFile; fileRef = 566DDE0C288D763C0000DCFB /* Fixits.swift */; };
56703297212B5450007D270F /* DatabaseUUIDEncodingStrategyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56703290212B544F007D270F /* DatabaseUUIDEncodingStrategyTests.swift */; };
56713FDD2691F409006153C3 /* JSONRequiredEncoder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 56713FDC2691F409006153C3 /* JSONRequiredEncoder.swift */; };
Expand Down Expand Up @@ -534,6 +535,7 @@
563C67B224628BEA00E94EDC /* DatabasePoolTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabasePoolTests.swift; sourceTree = "<group>"; };
563CBBE02A595131008905CE /* SQLIndexGenerator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SQLIndexGenerator.swift; sourceTree = "<group>"; };
563DE4EC231A91E2005081B7 /* DatabaseConfigurationTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabaseConfigurationTests.swift; sourceTree = "<group>"; };
563EA3E02C7B3A22001BE0D4 /* Mutex.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Mutex.swift; sourceTree = "<group>"; };
563EF414215F87EB007DAACD /* OrderedDictionary.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OrderedDictionary.swift; sourceTree = "<group>"; };
563EF42C2161180D007DAACD /* AssociationAggregate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AssociationAggregate.swift; sourceTree = "<group>"; };
563EF43E216131D1007DAACD /* AssociationAggregateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AssociationAggregateTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -628,7 +630,7 @@
566B912A1FA4D0CC0012D5B0 /* StatementAuthorizer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatementAuthorizer.swift; sourceTree = "<group>"; };
566B91321FA4D3810012D5B0 /* TransactionObserver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TransactionObserver.swift; sourceTree = "<group>"; };
566B9C1F25C6CC24004542CF /* RowDecodingError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RowDecodingError.swift; sourceTree = "<group>"; };
566BE7172342542F00A8254B /* LockedBox.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LockedBox.swift; sourceTree = "<group>"; };
566BE7172342542F00A8254B /* Mutex.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Mutex.swift; sourceTree = "<group>"; };
566DDE0C288D763C0000DCFB /* Fixits.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Fixits.swift; sourceTree = "<group>"; };
56703290212B544F007D270F /* DatabaseUUIDEncodingStrategyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DatabaseUUIDEncodingStrategyTests.swift; sourceTree = "<group>"; };
56713FDC2691F409006153C3 /* JSONRequiredEncoder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = JSONRequiredEncoder.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1010,6 +1012,7 @@
children = (
56677C14241D14450050755D /* FailureTestCase.swift */,
5623E0901B4AFACC00B20B7F /* GRDBTestCase.swift */,
563EA3E02C7B3A22001BE0D4 /* Mutex.swift */,
562EA81E1F17B26F00FA528C /* Compilation */,
56A238111B9C74A90082EB20 /* Core */,
567B5BDA2AD3281B00629622 /* Dump */,
Expand Down Expand Up @@ -1318,7 +1321,7 @@
56717270261C68E900423B6F /* CaseInsensitiveIdentifier.swift */,
563EF4492161F179007DAACD /* Inflections.swift */,
569BBA482291707D00478429 /* Inflections+English.swift */,
566BE7172342542F00A8254B /* LockedBox.swift */,
566BE7172342542F00A8254B /* Mutex.swift */,
563B8FC424A1D3B9007A48C9 /* OnDemandFuture.swift */,
563EF414215F87EB007DAACD /* OrderedDictionary.swift */,
5659F4971EA8D989004A4992 /* Pool.swift */,
Expand Down Expand Up @@ -2058,6 +2061,7 @@
5615B275222B107900061C1C /* AssociationHasOneThroughFetchableRecordTests.swift in Sources */,
56D4966F1D81309E008276D7 /* RecordPrimaryKeyNoneTests.swift in Sources */,
56D496621D81304E008276D7 /* StatementArguments+FoundationTests.swift in Sources */,
563EA3E12C7B3A22001BE0D4 /* Mutex.swift in Sources */,
56176C5D1EACCCC7000F3F2B /* FTS5TokenizerTests.swift in Sources */,
56D496B21D8133CE008276D7 /* DatabaseQueueInMemoryTests.swift in Sources */,
562393601DEE06D300A6B01F /* CursorTests.swift in Sources */,
Expand Down Expand Up @@ -2265,7 +2269,7 @@
5690C3401D23E82A00E59934 /* Data.swift in Sources */,
5659F4881EA8D94E004A4992 /* Utils.swift in Sources */,
567B5BE82AD3284100629622 /* DumpFormat.swift in Sources */,
566BE71E2342542F00A8254B /* LockedBox.swift in Sources */,
566BE71E2342542F00A8254B /* Mutex.swift in Sources */,
56A238931B9C750B0082EB20 /* DatabaseMigrator.swift in Sources */,
5603CEBB2AC862EC00CF097D /* SQLJSONExpressible.swift in Sources */,
56F89DF72A57EAA9002FE2AA /* ColumnDefinition.swift in Sources */,
Expand Down
42 changes: 25 additions & 17 deletions GRDB/Core/Database.swift
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
var isInsideTransactionBlock = false

/// Support for `checkForSuspensionViolation(from:)`
@LockedBox var isSuspended = false
let isSuspendedMutex = Mutex(false)

/// Support for `checkForSuspensionViolation(from:)`
/// This cache is never cleared: we assume journal mode never changes.
Expand Down Expand Up @@ -1125,22 +1125,25 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
///
/// Suspension ends with `resume()`.
func suspend() {
$isSuspended.update { isSuspended in
let needsInterrupt = isSuspendedMutex.withLock { isSuspended in
if isSuspended {
return
return false
}

// Prevent future lock acquisition
isSuspended = true

// Interrupt the database because this may trigger an
// SQLITE_INTERRUPT error which may itself abort a transaction and
// release a lock. See <https://www.sqlite.org/c3ref/interrupt.html>
return true
}

if needsInterrupt {
// Interrupting the database can trigger an SQLITE_INTERRUPT
// error which may itself abort a transaction and
// release a database lock, which is our goal.
// See <https://www.sqlite.org/c3ref/interrupt.html>
//
// Maybe interrupt will not release any lock. To address this,
// we'll issue a rollback on next database access which requires
// a lock. See `checkForSuspensionViolation(from:).`
interrupt()

// Now what about the eventual remaining lock? We'll issue a
// rollback on next database access which requires a lock, in
// checkForSuspensionViolation(from:).
}
}

Expand All @@ -1152,7 +1155,7 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
///
/// See suspend().
func resume() {
isSuspended = false
isSuspendedMutex.store(false)
}

/// Support for `checkForSuspensionViolation(from:)`
Expand Down Expand Up @@ -1182,9 +1185,9 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
///
/// See `suspend()` and ``Configuration/observesSuspensionNotifications``.
func checkForSuspensionViolation(from statement: Statement) throws {
try $isSuspended.read { isSuspended in
let needsAbort = try isSuspendedMutex.withLock { isSuspended in
guard isSuspended else {
return
return false
}

if try journalMode() == "wal" && statement.isReadonly {
Expand All @@ -1195,7 +1198,7 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
// Those are not read-only:
// - INSERT ...
// - BEGIN IMMEDIATE TRANSACTION
return
return false
}

if statement.releasesDatabaseLock {
Expand All @@ -1204,9 +1207,14 @@ public final class Database: CustomStringConvertible, CustomDebugStringConvertib
// - ROLLBACK
// - ROLLBACK TRANSACTION TO SAVEPOINT
// - RELEASE SAVEPOINT
return
return false
}

// Assume statement can acquire a write lock: abort.
return true
}

if needsAbort {
// Attempt at releasing an eventual lock with ROLLBACk,
// as explained in Database.suspend().
//
Expand Down
6 changes: 3 additions & 3 deletions GRDB/Core/DatabasePool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public final class DatabasePool {
/// It is constant, until close() sets it to nil.
private var readerPool: Pool<SerializedDatabase>?

@LockedBox var databaseSnapshotCount = 0
let databaseSnapshotCountMutex = Mutex(0)

/// If Database Suspension is enabled, this array contains the necessary `NotificationCenter` observers.
private var suspensionObservers: [NSObjectProtocol] = []
Expand Down Expand Up @@ -144,7 +144,7 @@ public final class DatabasePool {
}
}

// @unchecked because of databaseSnapshotCount, readerPool and suspensionObservers
// @unchecked because of readerPool and suspensionObservers
extension DatabasePool: @unchecked Sendable { }

extension DatabasePool {
Expand Down Expand Up @@ -845,7 +845,7 @@ extension DatabasePool {
path: path,
configuration: DatabasePool.readerConfiguration(writer.configuration),
defaultLabel: "GRDB.DatabasePool",
purpose: "snapshot.\($databaseSnapshotCount.increment())")
purpose: "snapshot.\(databaseSnapshotCountMutex.increment())")
}

#if SQLITE_ENABLE_SNAPSHOT || (!GRDBCUSTOMSQLITE && !GRDBCIPHER)
Expand Down
10 changes: 5 additions & 5 deletions GRDB/Core/DatabaseRegionObservation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -89,16 +89,16 @@ extension DatabaseRegionObservation {
onChange: @escaping (Database) -> Void)
-> AnyDatabaseCancellable
{
@LockedBox var state = ObservationState.pending
let stateMutex = Mutex(ObservationState.pending)

// Use unsafeReentrantWrite so that observation can start from any
// dispatch queue.
writer.unsafeReentrantWrite { db in
do {
let region = try observedRegion(db).observableRegion(db)
$state.update {
stateMutex.withLock { state in
let observer = DatabaseRegionObserver(region: region, onChange: {
if case .cancelled = state {
if case .cancelled = stateMutex.load() {
return
}
onChange($0)
Expand All @@ -111,7 +111,7 @@ extension DatabaseRegionObservation {
// the observer.
db.add(transactionObserver: observer, extent: .observerLifetime)

$0 = .started(observer)
state = .started(observer)
}
} catch {
onError(error)
Expand All @@ -122,7 +122,7 @@ extension DatabaseRegionObservation {
// Deallocates the transaction observer. This makes sure that the
// `onChange` callback will never be called again, because the
// observation was started with the `.observerLifetime` extent.
state = .cancelled
stateMutex.store(.cancelled)
}
}
}
Expand Down
72 changes: 0 additions & 72 deletions GRDB/Utils/LockedBox.swift

This file was deleted.

54 changes: 54 additions & 0 deletions GRDB/Utils/Mutex.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import Foundation

/// A Mutex protects a value with an NSLock.
///
/// We'll replace it with the SE-0433 Mutex when it is available.
/// <https://github.com/apple/swift-evolution/blob/main/proposals/0433-mutex.md>
final class Mutex<T> {
private var _value: T
private var lock = NSLock()

init(_ value: T) {
_value = value
}

/// Runs the provided closure while holding a lock on the value.
///
/// - parameter body: A closure that can modify the value.
func withLock<U>(_ body: (inout T) throws -> U) rethrows -> U {
lock.lock()
defer { lock.unlock() }
return try body(&_value)
}
}

// Inspired by <https://forums.swift.org/t/se-0433-synchronous-mutual-exclusion-lock/71174/58>
extension Mutex {
func load() -> T {
withLock { $0 }
}

func store(_ value: T) {
withLock { $0 = value }
}
}

extension Mutex where T: Numeric {
@discardableResult
func increment() -> T {
withLock { n in
n += 1
return n
}
}

@discardableResult
func decrement() -> T {
withLock { n in
n -= 1
return n
}
}
}

extension Mutex: @unchecked Sendable where T: Sendable { }
6 changes: 3 additions & 3 deletions GRDB/ValueObservation/Observers/ValueConcurrentObserver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ final class ValueConcurrentObserver<Reducer: ValueReducer, Scheduler: ValueObser
private var notificationCallbacks: NotificationCallbacks?

/// The fetching state for observation of constant regions.
@LockedBox private var fetchingState = FetchingState.idle
private let fetchingStateMutex = Mutex(FetchingState.idle)

/// Support for `TransactionObserver`, protected by the serialized writer
/// dispatch queue.
Expand Down Expand Up @@ -780,7 +780,7 @@ extension ValueConcurrentObserver: TransactionObserver {
}

private func setNeedsFetching(databaseAccess: DatabaseAccess) {
$fetchingState.update { state in
fetchingStateMutex.withLock { state in
switch state {
case .idle:
state = .fetching
Expand All @@ -806,7 +806,7 @@ extension ValueConcurrentObserver: TransactionObserver {

self.reduce(fetchResult)

$fetchingState.update { state in
fetchingStateMutex.withLock { state in
switch state {
case .idle:
// GRDB bug
Expand Down
Loading