Grdb.swift: Problems with select inside other select

Created on 22 Jul 2017  路  9Comments  路  Source: groue/GRDB.swift

Hi,

Im trying fill an object with a result of other select, but inside first object select.

Look:

    static func findAllWithMyProduct() -> [Product] {
        var list = [Product]()

        SharedData.shared.db.read { db in
            if let rows = try? Row.fetchAll(db, "SELECT p.* FROM product p INNER JOIN my_product mp ON mp.catalog_id = p.catalog_id ORDER BY mp.created_at DESC", arguments: []) {
                for x in 0..<rows.count {
                    let product = ProductService.bindFromRow(row: rows[x])
                    product.libraries = ProductLibraryService.findAllByCatalogId(catalogId: product.catalogId) // crash happens here
                    list.append(product)
                }
            }
        }

        return list
    }

Error:

fatal error: Database methods are not reentrant.: file /Users/paulo/Developer/workspaces/xcode/ubook-reader-ios/grdb/GRDB/Core/SerializedDatabase.swift, line 89
question

Most helpful comment

Hello @prsolucoes,

The "Database methods are not reentrant" fatal error means that it is a programmer error to call a database method from a block run by another database method:

// Don't do that
connection.read { db in
    connection.read { db in
        ...
    }
}

This is a programmer error for reasons that are all related to multithreading safety.

For example, consider your workaround:

// Your current way to avoid the crash:
let products = ProductLibrary.findAllWithMyProduct()
for product in list {
    product.libraries = ProductLibraryService.findAllByCatalogId(catalogId: product.catalogId)
}

This code is unsafe. What happens if a background thread alters products or libraries in the database between your fetches? You have a high risk of loading garbage or inconsistent data:

let products = ProductLibrary.findAllWithMyProduct()
// <- a background thread alters the database here
for product in list {
    product.libraries = ProductLibraryService.findAllByCatalogId(catalogId: product.catalogId)
    // <- a background thread alters the database here
}

Your solution is the following: all methods that use the database should accept a Database argument. Don't use SharedData.shared.db inside those methods.

For example, your findAllWithMyProduct method could be rewritten like this:

extension ProductService {
    static func findAllWithMyProduct(_ db: Database) throws -> [Product] {
        var list = [Product]()
        let rows = try Row.fetchAll(db, "SELECT p.* FROM product p INNER JOIN my_product mp ON mp.catalog_id = p.catalog_id ORDER BY mp.created_at DESC")
        for row in rows {
            let product = ProductService.bindFromRow(row: row)
            product.libraries = try ProductLibraryService.findAllByCatalogId(db, catalogId: product.catalogId)
            list.append(product)
        }
        return list
    }
}

extension ProductLibraryService {
    static func findAllByCatalogId(_ db: Database, catalogId: Int) throws -> [ProductLibrary] {
        return ...
    }
}

And you would call it this way:

let products = try SharedData.shared.db.read { db in
    try ProductService.findAllWithMyProduct(db)
}

You could also write it:

let products = try SharedData.shared.db.read { db -> [Product] in
    let products = try ProductService.findAllWithMyProduct(db)
    for product in products {
        product.libraries = try ProductLibraryService.findAllByCatalogId(db, catalogId: product.catalogId)
    }
    return products
}

In the last example above, background threads have absolutely no way to mess with your database requests. Even if you run many requests. Your application is guaranteed to load consistent data.

Yes, this means that your application controllers will have to wrap all database accesses in SharedData.shared.db... blocks, as above.

You may find it verbose. But this is the only way for your application to have the absolute guarantee that the data it fetches is consistent, regardless of eventual database updates performed by other parts of your application. This may look as an inconvenience first, but you'll enjoy the safety in the long term, as your applications grow in scope and complexity.

If you want to go further, please refer to:

All 9 comments

What i make to work is move:

product.libraries = ProductLibraryService.findAllByCatalogId(catalogId: product.catalogId) // crash happens here

To a loop outside the main loop:

    for product in list {
            product.libraries = ProductLibraryService.findAllByCatalogId(catalogId: product.catalogId)
        }

Hello @prsolucoes,

The "Database methods are not reentrant" fatal error means that it is a programmer error to call a database method from a block run by another database method:

// Don't do that
connection.read { db in
    connection.read { db in
        ...
    }
}

This is a programmer error for reasons that are all related to multithreading safety.

For example, consider your workaround:

// Your current way to avoid the crash:
let products = ProductLibrary.findAllWithMyProduct()
for product in list {
    product.libraries = ProductLibraryService.findAllByCatalogId(catalogId: product.catalogId)
}

This code is unsafe. What happens if a background thread alters products or libraries in the database between your fetches? You have a high risk of loading garbage or inconsistent data:

let products = ProductLibrary.findAllWithMyProduct()
// <- a background thread alters the database here
for product in list {
    product.libraries = ProductLibraryService.findAllByCatalogId(catalogId: product.catalogId)
    // <- a background thread alters the database here
}

Your solution is the following: all methods that use the database should accept a Database argument. Don't use SharedData.shared.db inside those methods.

For example, your findAllWithMyProduct method could be rewritten like this:

extension ProductService {
    static func findAllWithMyProduct(_ db: Database) throws -> [Product] {
        var list = [Product]()
        let rows = try Row.fetchAll(db, "SELECT p.* FROM product p INNER JOIN my_product mp ON mp.catalog_id = p.catalog_id ORDER BY mp.created_at DESC")
        for row in rows {
            let product = ProductService.bindFromRow(row: row)
            product.libraries = try ProductLibraryService.findAllByCatalogId(db, catalogId: product.catalogId)
            list.append(product)
        }
        return list
    }
}

extension ProductLibraryService {
    static func findAllByCatalogId(_ db: Database, catalogId: Int) throws -> [ProductLibrary] {
        return ...
    }
}

And you would call it this way:

let products = try SharedData.shared.db.read { db in
    try ProductService.findAllWithMyProduct(db)
}

You could also write it:

let products = try SharedData.shared.db.read { db -> [Product] in
    let products = try ProductService.findAllWithMyProduct(db)
    for product in products {
        product.libraries = try ProductLibraryService.findAllByCatalogId(db, catalogId: product.catalogId)
    }
    return products
}

In the last example above, background threads have absolutely no way to mess with your database requests. Even if you run many requests. Your application is guaranteed to load consistent data.

Yes, this means that your application controllers will have to wrap all database accesses in SharedData.shared.db... blocks, as above.

You may find it verbose. But this is the only way for your application to have the absolute guarantee that the data it fetches is consistent, regardless of eventual database updates performed by other parts of your application. This may look as an inconvenience first, but you'll enjoy the safety in the long term, as your applications grow in scope and complexity.

If you want to go further, please refer to:

Im having another problem with a service that run inside a Timer and read/write/delete somethings that need be in sync with my server. But when the background service inside Timer is using database and i need insert or read data on my app when user is using it, it crash too.

What the solution for this?

Im connection with database using this code:

// configura莽茫o do banco de dados
do {
    let dbDirectoryPath = NSSearchPathForDirectoriesInDomains(.documentDirectory, .userDomainMask, true)[0]
    try FileManager.default.createDirectory(atPath: dbDirectoryPath, withIntermediateDirectories: true, attributes: nil)
    let dbPath = (dbDirectoryPath as NSString).appendingPathComponent("db.sqlite")

    NSLog("Caminho do banco de dados: %@", dbPath)

    SharedData.shared.db = try DatabaseQueue(path: dbPath)
    NSLog("Conectado ao banco de dados com sucesso")

    migrateDatabase()
} catch let error {
    NSLog("Erro ao conectar ao banco de dados: %@", error.localizedDescription)
}

I do what you describe here:

SharedData.shared.db.inDatabase({ db in
    EventService.add(db: db, event: Event(type: EventType.playInfo, data: [
        "catalog_id": self.product.catalogId ?? 0,
        "chapter_id": 0,
        "duration": 0,
        "event_time": Date(),
        "progress": (percentage * 100),
        ]))
})

Every method now use in this way.

static func add(db: Database, event: Event) {
    insert(db: db, event: event)
}

static func insert(db: Database, event: Event) {
    do {
        let statement = try db.makeUpdateStatement("" +
            "INSERT INTO event " +
            "(type, data, uploaded, created_at) " +
            "VALUES " +
            "(:type, :data, :uploaded, :created_at)")

        event.createdAt = Date()

        var arguments: StatementArguments = []
        _ = arguments.append(contentsOf: ["type": event.type ?? ""])
        _ = arguments.append(contentsOf: ["data": event.getDataAsJSON()])
        _ = arguments.append(contentsOf: ["uploaded": event.uploaded ?? false])
        _ = arguments.append(contentsOf: ["created_at": DatabaseTimestamp(event.createdAt)])

        statement.arguments = arguments

        try statement.execute()

        event.id = Int(exactly: db.lastInsertedRowID) ?? 0
    } catch let error {
        Logger.e("Erro ao executar query no banco de dados: %@", error.localizedDescription)
    }
}

The bug still happening.

invalid mode 'kCFRunLoopCommonModes' provided to CFRunLoopRunSpecific - break on _CFRunLoopError_RunCalledWithInvalidMode to debug. This message will only appear once per execution.

fatal error: Database methods are not reentrant.: file /Users/paulo/Developer/workspaces/xcode/project-ios/grdb/GRDB/Core/SerializedDatabase.swift, line 89
(lldb) 

Im executing it after a WebKit javascript call completionHandle:

webView.evaluateJavaScript(javascript, completionHandler: { (response: Any?, error: Error?) in
    if let currentError = error {
        Logger.e("Erro ao executar JS: %@", currentError.localizedDescription)
    }

    if let completionHandler = completionHandler {
        completionHandler(response, error)
    }
})

The problem dont appear happen in other places.

Please, help me with this. It is crashing the app a lot.

Hello @prsolucoes

I'm glad you have refactored your code. Now if you still have the crash, remember that you must not call SharedData.shared.db.inDatabase from SharedData.shared.db.inDatabase:

// No
SharedData.shared.db.inDatabase { db in
    SharedData.shared.db.inDatabase { db in
        ...
    }
}

So when a crash happens, open you eyes and look at the stack trace: it will show you two SharedData.shared.db.inDatabase. The goal is to remove the one which crashes. To remove it, replace it by the db that you pass along from the enclosing SharedData.shared.db.inDatabase.

Thanks for the help.

I solve the problem above.

Great! Happy GRDB!

Was this page helpful?
0 / 5 - 0 ratings