Merge pull request #555 from groue/dev/readOnlyError

Avoid a crash when read-only access can't be established
groue · Jun 19, 2019 · 49d098e · 49d098e
2 parents 2730f8f + caa624f
commit 49d098e
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -67,6 +67,7 @@ Released June 15, 2019 &bull; [diff](https://github.com/groue/GRDB.swift/compare
 - [#546](https://github.com/groue/GRDB.swift/pull/546) by [@robcas3](https://github.com/robcas3): Fix SPM errors with Xcode 11 beta
 - [#549](https://github.com/groue/GRDB.swift/pull/549) Support for Combine
 - [#550](https://github.com/groue/GRDB.swift/pull/550) Asynchronous Database Access Methods
+- [#555](https://github.com/groue/GRDB.swift/pull/555) Avoid a crash when read-only access can't be established
 
 ### Documentation Diff
 

diff --git a/GRDB/Core/Database.swift b/GRDB/Core/Database.swift
@@ -162,20 +162,7 @@ public final class Database {
 
     private var functions = Set<DatabaseFunction>()
     private var collations = Set<DatabaseCollation>()
-    private var readOnlyDepth = 0 {
-        didSet {
-            // query_only pragma was added in SQLite 3.8.0 http://www.sqlite.org/changes.html#version_3_8_0
-            // It is available from iOS 8.2 and OS X 10.10 https://github.com/yapstudios/YapDatabase/wiki/SQLite-version-(bundled-with-OS)
-            // Assume those pragmas never fail
-            switch (oldValue, readOnlyDepth) {
-            case (1, 0):
-                try! internalCachedUpdateStatement(sql: "PRAGMA query_only = 0").execute()
-            case (0, 1):
-                try! internalCachedUpdateStatement(sql: "PRAGMA query_only = 1").execute()
-            default: break
-            }
-        }
-    }
+    private var _readOnlyDepth = 0 // Modify with beginReadOnly/endReadOnly
     private var isClosed: Bool = false
 
     // MARK: - Initializer
@@ -548,15 +535,50 @@ extension Database {
 
     // MARK: - Read-Only Access
 
+    func beginReadOnly() throws {
+        _readOnlyDepth += 1
+        if _readOnlyDepth == 1 && configuration.readonly == false  {
+            // PRAGMA query_only was added in SQLite 3.8.0 http://www.sqlite.org/changes.html#version_3_8_0
+            // It is available from iOS 8.2 and OS X 10.10 https://github.com/yapstudios/YapDatabase/wiki/SQLite-version-(bundled-with-OS)
+            try internalCachedUpdateStatement(sql: "PRAGMA query_only = 1").execute()
+        }
+    }
+
+    func endReadOnly() throws {
+        _readOnlyDepth -= 1
+        if _readOnlyDepth == 0 && configuration.readonly == false {
+            // PRAGMA query_only was added in SQLite 3.8.0 http://www.sqlite.org/changes.html#version_3_8_0
+            // It is available from iOS 8.2 and OS X 10.10 https://github.com/yapstudios/YapDatabase/wiki/SQLite-version-(bundled-with-OS)
+            try internalCachedUpdateStatement(sql: "PRAGMA query_only = 0").execute()
+        }
+    }
+
     /// Grants read-only access, starting SQLite 3.8.0
-    func readOnly<T>(_ block: () throws -> T) rethrows -> T {
-        if configuration.readonly {
-            return try block()
+    func readOnly<T>(_ block: () throws -> T) throws -> T {
+        try beginReadOnly()
+
+        var result: T?
+        var thrownError: Error?
+
+        do {
+            result = try block()
+        } catch {
+            thrownError = error
+        }
+
+        do {
+            try endReadOnly()
+        } catch {
+            if thrownError == nil {
+                thrownError = error
+            }
+        }
+
+        if let error = thrownError {
+            throw error
         }
 
-        readOnlyDepth += 1
-        defer { readOnlyDepth -= 1 }
-        return try block()
+        return result!
     }
 }
 

diff --git a/GRDB/Core/DatabaseQueue.swift b/GRDB/Core/DatabaseQueue.swift
@@ -188,7 +188,17 @@ extension DatabaseQueue {
     /// - parameter block: A block that accesses the database.
     public func asyncRead(_ block: @escaping (Result<Database, Error>) -> Void) {
         writer.async { db in
-            db.readOnly { block(.success(db)) }
+            do {
+                try db.beginReadOnly()
+            } catch {
+                block(.failure(error))
+                return
+            }
+
+            block(.success(db))
+
+            // Ignore error because we can not notify it.
+            try? db.endReadOnly()
         }
     }
     #endif
@@ -249,9 +259,18 @@ extension DatabaseQueue {
         writer.execute { db in
             // ... and that no transaction is opened.
             GRDBPrecondition(!db.isInsideTransaction, "must not be called from inside a transaction.")
-            db.readOnly {
-                block(.success(db))
+
+            do {
+                try db.beginReadOnly()
+            } catch {
+                block(.failure(error))
+                return
             }
+
+            block(.success(db))
+
+            // Ignore error because we can not notify it.
+            try? db.endReadOnly()
         }
     }
     #endif