Have cursors reset SQLite statements in deinit

CHANGELOG.md

@@ -6,6 +6,7 @@ Release Notes
 ### Fixed
 
 - [#425](https://github.com/groue/GRDB.swift/pull/425): Fix "destination database is in use " error during database backup
+- [#427](https://github.com/groue/GRDB.swift/pull/427): Have cursors reset SQLite statements in deinit
 
 
 ### New

GRDB/Core/Database.swift

@@ -778,16 +778,6 @@ extension Database {
     // MARK: - Backup
 
     static func backup(from dbFrom: Database, to dbDest: Database, afterBackupInit: (() -> ())? = nil, afterBackupStep: (() -> ())? = nil) throws {
-        // The schema of the destination database will change: we need to clear
-        // the schema and statements cache.
-        //
-        // We especially need to clear the statements cache before calling
-        // sqlite3_backup_init, so that we avoid an SQLite error
-        // "destination database is in use" (SQLITE_ERROR).
-        //
-        // See https://github.com/groue/GRDB.swift/issues/424
-        dbDest.clearSchemaCache()
-
         guard let backup = sqlite3_backup_init(dbDest.sqliteConnection, "main", dbFrom.sqliteConnection, "main") else {
             throw DatabaseError(resultCode: dbDest.lastErrorCode, message: dbDest.lastErrorMessage)
         }
@@ -820,6 +810,9 @@ extension Database {
         case let code:
             throw DatabaseError(resultCode: code, message: dbDest.lastErrorMessage)
         }
+
+        // The schema of the destination database has changed:
+        dbDest.clearSchemaCache()
     }
 }
 

GRDB/Core/DatabaseValueConvertible.swift

@@ -65,6 +65,12 @@ public final class DatabaseValueCursor<Value: DatabaseValueConvertible> : Cursor
         statement.reset(withArguments: arguments)
     }
 
+    deinit {
+        // Statement reset fails when sqlite3_step has previously failed.
+        // Just ignore reset error.
+        try? statement.reset()
+    }
+
     /// :nodoc:
     public func next() throws -> Value? {
         if done {
@@ -119,6 +125,12 @@ public final class NullableDatabaseValueCursor<Value: DatabaseValueConvertible>
         statement.reset(withArguments: arguments)
     }
 
+    deinit {
+        // Statement reset fails when sqlite3_step has previously failed.
+        // Just ignore reset error.
+        try? statement.reset()
+    }
+
     /// :nodoc:
     public func next() throws -> Value?? {
         if done {

GRDB/Core/DatabaseWriter.swift

@@ -153,10 +153,6 @@ extension DatabaseWriter {
         // SQLCipher does not support the backup API: https://discuss.zetetic.net/t/using-the-sqlite-online-backup-api/2631
         // So we'll drop all database objects one after the other.
         try writeWithoutTransaction { db in
-            // Clear schema and statement cache so that no prepared statement
-            // can prevent schema change.
-            db.clearSchemaCache()
-
             // Prevent foreign keys from messing with drop table statements
             let foreignKeysEnabled = try Bool.fetchOne(db, "PRAGMA foreign_keys")!
             if foreignKeysEnabled {

GRDB/Core/Row.swift

@@ -617,6 +617,12 @@ public final class RowCursor : Cursor {
         statement.reset(withArguments: arguments)
     }
 
+    deinit {
+        // Statement reset fails when sqlite3_step has previously failed.
+        // Just ignore reset error.
+        try? statement.reset()
+    }
+
     /// :nodoc:
     public func next() throws -> Row? {
         if done {
@@ -702,8 +708,11 @@ extension Row {
     /// - returns: An optional row.
     /// - throws: A DatabaseError is thrown whenever an SQLite error occurs.
     public static func fetchOne(_ statement: SelectStatement, arguments: StatementArguments? = nil, adapter: RowAdapter? = nil) throws -> Row? {
-        // The cursor reuses a single mutable row. Return an immutable copy.
-        return try fetchCursor(statement, arguments: arguments, adapter: adapter).next().map { $0.copy() }
+        let cursor = try fetchCursor(statement, arguments: arguments, adapter: adapter)
+        // Keep cursor alive until we can copy the fetched row
+        return try withExtendedLifetime(cursor) {
+            try cursor.next().map { $0.copy() }
+        }
     }
 }
 

GRDB/Core/Statement.swift

@@ -83,14 +83,11 @@ public class Statement {
         sqlite3_finalize(sqliteStatement)
     }
 
-    final func reset() {
-        // It looks like sqlite3_reset() does not access the file system.
-        // This function call should thus succeed, unless a GRDB bug, or a
-        // programmer error (reusing a failed statement): there is no point
-        // throwing any error.
+    final func reset() throws {
+        SchedulingWatchdog.preconditionValidQueue(database)
         let code = sqlite3_reset(sqliteStatement)
         guard code == SQLITE_OK else {
-            fatalError(DatabaseError(resultCode: code, message: database.lastErrorMessage, sql: sql).description)
+            throw DatabaseError(resultCode: code, message: database.lastErrorMessage, sql: sql)
         }
     }
 
@@ -136,7 +133,7 @@ public class Statement {
         _arguments = arguments
         argumentsNeedValidation = false
 
-        reset()
+        try! reset()
         clearBindings()
 
         var valuesIterator = arguments.values.makeIterator()
@@ -159,7 +156,7 @@ public class Statement {
         argumentsNeedValidation = false
 
         // Apply
-        reset()
+        try reset()
         clearBindings()
         for (index, dbValue) in zip(Int32(1)..., bindings) {
             bind(dbValue, at: index)
@@ -363,9 +360,8 @@ public final class SelectStatement : Statement {
 
     /// Utility function for cursors
     func reset(withArguments arguments: StatementArguments? = nil) {
-        SchedulingWatchdog.preconditionValidQueue(database)
         prepare(withArguments: arguments)
-        reset()
+        try! reset()
     }
 }
 
@@ -392,6 +388,12 @@ public final class StatementCursor: Cursor {
         statement.reset(withArguments: arguments)
     }
 
+    deinit {
+        // Statement reset fails when sqlite3_step has previously failed.
+        // Just ignore reset error.
+        try? statement.reset()
+    }
+
     /// :nodoc:
     public func next() throws -> Void? {
         if done {
@@ -480,7 +482,7 @@ public final class UpdateStatement : Statement {
     public func execute(arguments: StatementArguments? = nil) throws {
         SchedulingWatchdog.preconditionValidQueue(database)
         prepare(withArguments: arguments)
-        reset()
+        try reset()
         database.updateStatementWillExecute(self)
 
         while true {

GRDB/Core/StatementColumnConvertible.swift

@@ -77,6 +77,12 @@ public final class FastDatabaseValueCursor<Value: DatabaseValueConvertible & Sta
         statement.reset(withArguments: arguments)
     }
 
+    deinit {
+        // Statement reset fails when sqlite3_step has previously failed.
+        // Just ignore reset error.
+        try? statement.reset()
+    }
+
     /// :nodoc:
     public func next() throws -> Value? {
         if done {
@@ -127,6 +133,12 @@ public final class FastNullableDatabaseValueCursor<Value: DatabaseValueConvertib
         statement.reset(withArguments: arguments)
     }
 
+    deinit {
+        // Statement reset fails when sqlite3_step has previously failed.
+        // Just ignore reset error.
+        try? statement.reset()
+    }
+
     /// :nodoc:
     public func next() throws -> Value?? {
         if done {

GRDB/Record/FetchableRecord.swift

@@ -381,6 +381,12 @@ public final class RecordCursor<Record: FetchableRecord> : Cursor {
         statement.reset(withArguments: arguments)
     }
 
+    deinit {
+        // Statement reset fails when sqlite3_step has previously failed.
+        // Just ignore reset error.
+        try? statement.reset()
+    }
+
     /// :nodoc:
     public func next() throws -> Record? {
         if done {

Tests/GRDBTests/DatabasePoolReleaseMemoryTests.swift

@@ -188,39 +188,6 @@ class DatabasePoolReleaseMemoryTests: GRDBTestCase {
         XCTAssertEqual(openConnectionCount, 0)
     }
 
-    // TODO: this test should be duplicated for all cursor types
-    func testDatabaseCursorRetainSQLiteConnection() throws {
-        let countQueue = DispatchQueue(label: "GRDB")
-        var openConnectionCount = 0
-
-        dbConfiguration.SQLiteConnectionDidOpen = {
-            countQueue.sync {
-                openConnectionCount += 1
-            }
-        }
-
-        dbConfiguration.SQLiteConnectionDidClose = {
-            countQueue.sync {
-                openConnectionCount -= 1
-            }
-        }
-
-        var cursor: FastDatabaseValueCursor<Int>? = nil
-        do {
-            try! makeDatabasePool().write { db in
-                try db.execute("CREATE TABLE items (id INTEGER PRIMARY KEY)")
-                try db.execute("INSERT INTO items (id) VALUES (NULL)")
-                try db.execute("INSERT INTO items (id) VALUES (NULL)")
-                cursor = try Int.fetchCursor(db, "SELECT id FROM items")
-                XCTAssertTrue(try cursor!.next() != nil)
-                XCTAssertEqual(openConnectionCount, 1)
-            }
-        }
-        XCTAssertEqual(openConnectionCount, 0)
-        XCTAssertTrue(try! cursor!.next() != nil)
-        XCTAssertTrue(try! cursor!.next() == nil)
-    }
-
     func testStatementDoNotRetainDatabaseConnection() throws {
         // Block 1                  Block 2
         //                          create statement INSERT

Tests/GRDBTests/DatabaseQueueReleaseMemoryTests.swift

@@ -102,39 +102,6 @@ class DatabaseQueueReleaseMemoryTests: GRDBTestCase {
         // All connections are closed
         XCTAssertEqual(openConnectionCount, 0)
     }
-
-    // TODO: this test should be duplicated for all cursor types
-    func testDatabaseCursorRetainSQLiteConnection() throws {
-        let countQueue = DispatchQueue(label: "GRDB")
-        var openConnectionCount = 0
-
-        dbConfiguration.SQLiteConnectionDidOpen = {
-            countQueue.sync {
-                openConnectionCount += 1
-            }
-        }
-
-        dbConfiguration.SQLiteConnectionDidClose = {
-            countQueue.sync {
-                openConnectionCount -= 1
-            }
-        }
-
-        var cursor: FastDatabaseValueCursor<Int>? = nil
-        do {
-            try! makeDatabaseQueue().inDatabase { db in
-                try db.execute("CREATE TABLE items (id INTEGER PRIMARY KEY)")
-                try db.execute("INSERT INTO items (id) VALUES (NULL)")
-                try db.execute("INSERT INTO items (id) VALUES (NULL)")
-                cursor = try Int.fetchCursor(db, "SELECT id FROM items")
-                XCTAssertTrue(try cursor!.next() != nil)
-                XCTAssertEqual(openConnectionCount, 1)
-            }
-        }
-        XCTAssertEqual(openConnectionCount, 0)
-        XCTAssertTrue(try! cursor!.next() != nil)
-        XCTAssertTrue(try! cursor!.next() == nil)
-    }
 
     func testStatementDoNotRetainDatabaseConnection() throws {
         // Block 1                  Block 2