stephencelis / SQLite.swift

A type-safe, Swift-language layer over SQLite3.
MIT License
9.73k stars 1.57k forks source link

Crash when iOS suspends app after being backgrounded #1153

Open PatrickDanino opened 2 years ago

PatrickDanino commented 2 years ago

Build Information

v0.13.3 via SPM

Issue

We noticed a number of crashes due to EXC_CRASH: RUNNINGBOARD 0xdead10cc. According to the Apple docs, this is likely due to iOS terminating our app because we are actively performing database operations while in the background.

The problem is that we didn't initially know we needed to do anything special in order to avoid this problem, and this could easily happen to anyone who uses this framework.

One potential solution we are in the process of experimenting with is a helper function on the Connection object. Something like this:

@discardableResult
public func withThrowingBlock<T>(_ function: String = #function, _ block: (() throws -> T)) throws -> T {

    // Allows the block to be performed in the background for a short period of time
    let taskId = UIApplication.shared.beginBackgroundTask(withName: function)
    defer { UIApplication.shared.endBackgroundTask(taskId) }

    return try block()
}

This would make for a fairly simple solution to prevent crashes in the vast majority of cases. To use the example for this project:

do {
    let db = try Connection("path/to/db.sqlite3")
    let users = Table("users")
    let id = Expression<Int64>("id")
    let name = Expression<String?>("name")
    let email = Expression<String>("email")

    try db.withThrowingBlock {
        try db.run(users.create { t in
            t.column(id, primaryKey: true)
            t.column(name)
            t.column(email, unique: true)
        })
    }

} catch {
    print (error)
}

Or you could use it to return a value directly:

do {
    let db = try Connection("path/to/db.sqlite3")
    let users = Table("users")
    ...

    let count = try db.withThrowingBlock {
         try db.scalar(users.count) // 0
    }
    print(count)

} catch {
    print (error)
}
PatrickDanino commented 2 years ago

As a follow up, we ended up wrapping the entire SQLite Connection object into its own client, which mostly acts as a helper to ensure no one creates one manually. This also helps us with managing the lifetime of the connection as it appears to maintain a reference and needs to be properly closed.

The client class looks something like this:

/// Wraps a SQL database and allows for operations to be performed safely.
public class SQLiteClient {

    let databaseFilename: String?
    let logger: Logger

    private(set) var didEnterBackground = false

    private var cancellables = Set<AnyCancellable>()
    private var _connection: Connection?
    private var createTables: ((_ connection: Connection) throws -> Void)?

    // Custom path we use to store databases - needs to allow read/writes
    private static let databaseDirectory = FileManager.default.sharedDocumentsURL

    /// Initializes the SQL wrapper. You must either pass in a block to create the tables or
    /// call ``setup(createTables:)`` once at a later time.
    /// - Parameters:
    ///   - filename: The short name of the database. Example: "History.sqlite3"
    ///   - logger: The OS logger to which this client should log, usually a local logger.
    ///   - createTables: Optional. A block to immediately setup the database by creating tables only if they do not exist.
    ///   Do not call ``setup(createTables:)`` if used.
    public init(filename: String, logger: Logger, createTables: ((_ connection: Connection) throws -> Void)? = nil) {
        self.databaseFilename = Self.databaseDirectory?.appendingPathComponent(filename).path
        self.logger = logger

        // Track if running in the background
        subscribeToPublishers()

        // Try to setup if a block is passed in
        if let createTables = createTables {
            setup(createTables: createTables)
        }
    }

    // MARK: Public

    /// Clears the database by either deleting the specified tables,
    /// or deleting the database file if needed.
    /// - Parameter tables: Optional. An array of `Table` items to clear.
    /// If empty, the database file will be deleted.
    /// - Returns: True of clearing was successful, False if not.
    public func clearAll(tables: [Table] = []) -> Bool {

        // Try deleting the tables
        var didClear = deleteTables(tables)

        // Try deleting the database if needed
        if !didClear {
            didClear = deleteDatabase()
        }

        // Create a new database or tables if needed
        if didClear {
            createDatabaseIfNeeded()
        }

        return didClear
    }

    /// Deletes the underlying database permanently.
    /// Public for testing purposes.
    @discardableResult
    public func deleteDatabase() -> Bool {
        guard let databaseFilename = self.databaseFilename,
              FileManager.default.fileExists(atPath: databaseFilename)
        else { return false }

        do {
            // Close the connection before deleting the database
            self.connection = nil

            // Try deleting the database altogether
            try FileManager.default.removeItem(atPath: databaseFilename)

            return true

        } catch {
            log(error: error, message: "Error deleting database")
        }

        return false
    }

    /// Optional if a `createTables` block is not used during initialization.
    /// Setup the client by creating tables, but only if needed.
    /// Used if you need to reference `self` in the block in order to perform additional operations
    /// once the tables are properly created.
    /// - Parameter createTables: Block to create tables, but only do so if tables do not already exist.
    public func setup(createTables: @escaping ((_ connection: Connection) throws -> Void)) {
        guard !self.didSetup else { self.logger.resumableAssert("Setup should only be called once"); return }

        // Allows us to create or re-create tables as needed
        self.createTables = createTables

        // Creates the actual SQL database file and tables if needed
        createDatabaseIfNeeded()
    }

    /// Performs one or more operations on the connection in a safe way,
    /// particularly if the app is sent to the background.
    @discardableResult
    public func withSafeConnection<T>(_ block: ((Connection) throws -> T)) throws -> T {
        guard let connection = self.connection else { throw ServiceError.cancelled }

        do {
            return try connection.withSafeThrowingBlock {
                return try block(connection)
            }
        } catch {
            log(error: error, message: "Error performing operation")
            throw error
        }
    }

    // MARK: Private

    private var connection: Connection? {
        get {
            guard self.didSetup else { self.logger.resumableAssert("Setup must be called first"); return nil }
            guard !self.didEnterBackground else { return nil }

            if self._connection == nil {
                self._connection = createConnection()
            }

            return self._connection
        }
        set { self._connection = newValue }
    }

    private var didSetup: Bool { self.createTables != nil }

    private func createConnection() -> Connection? {
        guard let databaseFilename = self.databaseFilename
        else { self.logger.resumableAssert("Database file is missing"); return nil }

        var connection: Connection?

        do {
            connection = try Connection(databaseFilename)
        } catch {
            log(error: error, message: "Error creating connection")
        }

        self.logger.resumableAssert("Connection shouldn't be nil", connection != nil)
        return connection
    }

    private func createDatabaseIfNeeded() {
        do {
            try createDirectoryIfNeeded()
            try createTablesIfNeeded()
        } catch {
            log(error: error, message: "Error creating database")
            self._connection = nil
        }
    }

    private func createDirectoryIfNeeded() throws {
        guard let databaseFilename = self.databaseFilename else { throw ServiceError.invalidState }

        do {
            // Create documents directory if file does not exist
            if !FileManager.default.fileExists(atPath: databaseFilename),
               let databaseDirectoryPath = Self.databaseDirectory?.path {

                try FileManager.default.createDirectory(atPath: databaseDirectoryPath,
                                                        withIntermediateDirectories: true,
                                                        attributes: nil)
            }

        } catch {
            log(error: error, message: "Error creating directory")
            throw error
        }
    }

    private func createTablesIfNeeded() throws {
        guard let createTables = self.createTables else { throw ServiceError.invalidState }

        do {
            // Creates tables in a safe way
            try withSafeConnection { connection in
                try createTables(connection)
            }

        } catch {
            log(error: error, message: "Error creating tables")
            throw error
        }
    }

    /// Deletes the tables without deleting the actual database
    private func deleteTables(_ tables: [Table]) -> Bool {
        guard !tables.isEmpty else { return false }

        do {
            // Try dropping the database table
            try withSafeConnection { connection in
                try tables.forEach { table in
                    try connection.run(table.drop(ifExists: true))
                }

                return true
            }

        } catch {
            log(error: error, message: "Error clearing table")
        }

        return false
    }

    /// Logs a client error using the OS logger
    private func log(error: Error, message: StaticString) {
        let info = (error as? SQLite.QueryError)?.description
            ?? (error as? SQLite.Result)?.description
            ?? error.localizedDescription

        self.logger.error("\(message): \(info)")
    }

    private func subscribeToPublishers() {
        // Allows us to avoid initiating new operations while in the background
        NotificationCenter.default
            .publisher(for: UIApplication.willEnterForegroundNotification)
            .sink { [weak self] _ in
                self?.didEnterBackground = false
            }
            .store(in: &self.cancellables)

        NotificationCenter.default
            .publisher(for: UIApplication.didEnterBackgroundNotification)
            .sink { [weak self] _ in
                self?.didEnterBackground = true
            }
            .store(in: &self.cancellables)
    }
}

fileprivate extension Connection {

    /// Performs a block operation in a safe way by notifying
    /// iOS that it may need more time if the app is sent to the background.
    @discardableResult
    func withSafeThrowingBlock<T>(_ function: String = #function, _ block: (() throws -> T)) throws -> T {

        // Allows the block to be performed in the background for a short period of time
        let taskId = UIApplication.shared.beginBackgroundTask(withName: function)
        defer { UIApplication.shared.endBackgroundTask(taskId) }

        return try block()
    }
}

Here's an example of safe usage:

let logger = Logger(subsystem: Bundle.main.bundleIdentifier!, category: "cacheDatabase")
let client = SQLiteClient(filename: "cache.sqlite3", logger: logger)

// Create the database table if it doesn't exist
client.setup { connection in
    // Used to version the database schema
    connection.userVersion = 0

    // Creates tables only if needed
    try connection.run(CacheItem.tableCreate())
}

// Run a query
do {
    let count = return try client.withSafeConnection { connection in
        try connection.scalar(CacheItem.tableCount())
    }
    print("Table count: \(count)")
} catch {
    print("Error loading count: \(error.localizedDescription)")
}

// Clear data
if client.clearAll(tables: [CacheItem.table]) {
    // Clear succeeded
    postNotification()
}
PatrickDanino commented 1 year ago

There's no "official" way to test this, but if you wait 30 seconds, iOS will emit warnings and notify you about background tasks.

One issue that we did uncover since my original comment nearly a year ago is that despite what an Apple rep told me directly, there is a fairly high performance cost (or at least, noticeable) to performing a background task. As such, we only use this code when we must rather than always and unfortunately still experience crashes in some non-trivial scenarios where average performance matters more than occasional background issues.

BrettyWhite commented 7 months ago

So this was an odd issue for me. It started when we moved our database location to a shared app group container to be used by not only the app itself, but also a widget. I have background fetch and audio permissions so it wasn't a lack of permissions. I couldn't replicate the crash in any dev environment but it would happen randomly to other users, just with a pop-up saying that our app had crashed.

After many days of digging around I found this old gem: https://developer.apple.com/library/archive/technotes/tn2408/_index.html

In it it describes:

You can use CFPreferences, atomic safe save operations on flat files, or SQLite or Core Data to share data in a group container between multiple processes even if one is suspended mid transaction. For SQLite/Core Data, processes using databases in DELETE journal mode will be instantly killed instead of suspended. Processes using database in WAL journal mode will only be jetsam'd if they attempt to hold a write transaction open at the time of suspension. Posix file locks also work, and behave similarly to SQLite database in DELETE journal mode, so open(O_EXLOCK) or flock() could also be used.

Regardless of this issue, the containing app (and all applications) should properly use background task assertions around file operations they require completed in a shared container (with or without extensions). This includes all writes or deletions. Such a process might still be killed by jetsam but at a much lower frequency.

So for us:

    private func connectToDatabase(atPath path: String) -> Bool {
        do {
            db = try Connection(path)
            try db?.execute("PRAGMA foreign_keys = ON;")
            try db?.execute("PRAGMA journal_mode = WAL;") // Add this line to enable WAL
            return true
        } catch {
            print("Failed to establish a database connection: \(error)")
            return false
        }
    }

Adding try db?.execute("PRAGMA journal_mode = WAL;") fixed the issue for all of my users and the odd non-reproducible 0xdead10cc crashes went away.

@stephencelis this might be a good note for this part of the docs: https://github.com/stephencelis/SQLite.swift/blob/master/Documentation/Index.md#in-a-shared-group-container

Hope this can help someone else.