Transactions don't work with async/await

[aaronabramov]

I'm migrating a pretty big codebase to use async/await and ran into problem with making rusqlite work.
I understand why Connection is not Sync and normally i would use a Mutex to hold a connection in a multi threaded environment, but in this case i'm holding a reference to a Connection and &T: Send is not Send.

playground link: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=c25f81c48a319e5fbaf88b79ac1989eb

use rusqlite;
use tokio; // 0.2.13

fn main() {
    let rt = tokio::runtime::Runtime::new().unwrap();

    let handle = rt.spawn(async {
        let conn = rusqlite::Connection::open_in_memory().unwrap();
        let tx = conn.transaction().unwrap();
        async {
            // do something async
        }
        .await;
        tx.execute_batch("SELECT 1").unwrap();
        tx.commit();
    });

    rt.block_on(handle).unwrap();
}

[thomcc]

Hmm. In general, &T: Send tends to imply T: Sync -- if you can send a reference to something to another thread, it has to allow concurrent use. There are reasons we don't support this (see #342 -- some SQLite APIs are not thread safe, even when compiled with fully thread safety).

That said, I do think usability in async contexts is important to support, so I'll look into this.

[aaronabramov]

@thomcc that makes sense!
My current strategy is to begin/commit transactions manually (by sending BEGIN and COMMIT through regular Connection struct) while storing a Connection in a Mutex and locking/unlocking inside an async function when needed

[thomcc]

Are you able to use Statement across await? I'm guessing not... I know of a solution for Transaction (make transaction hold &mut Connection. Sadly this requires killing the new unchecked_transaction API, which has valid use cases... That said it's new so not that big of a deal I guess...)

It would be nice to fix the general problem though -- I suspect fixing that would mean many other APIs don't work still.

[Dushistov]

But what point to use blocking code (all sqlite code is blocking) in async context?
fsync may takes ages in some circumstances.

[KamilaBorowska]

rusqlite doesn't provide an async API, please use spawn_blocking.

Not using spawn_blocking is just asking for trouble, and with spawn_blocking things should just work.

[thomcc]

(This also roughly relates the question raised at #188 (comment) the other day).

From some discussion, I've heard that doing blocking operations on async worker threads is a big problem, and that it's probably good that rusqlite behave as it does currently, as a way to signal to users that they're doing something very wrong.

In the mean time, you should either follow @xfix's advice (use spawn_blocking), or if that is is not-viable, perhaps you could use the sqlx crate, which seems to make SQLite async by performing IO on a background thread.

Eventually there's a reasonable chance that similar async support will become desirable for rusqlite (I suspect Firefox may eventually need it...), but there are no short term plans for this. (And if this happens it wouldn't be the default though, it would be behind in a feature; or possibly even a separate rusqlite-async crate).

Because of all that I'm going to close this as works-as-intended for now, even if it's a bit surprising.

[broccolihighkicks]

@thomcc @xfix

If you use tokios task::spawn_blocking, isn't this the same problem (InnerConnection is not Send)?:

std::cell::RefCell<rusqlite::inner_connection::InnerConnection> cannot be shared between threads safely

[thomcc]

Connection should be send even if InnerConnection isn't (https://github.com/rusqlite/rusqlite/blob/master/src/lib.rs#L316). What API exposes the RefCell in a way where this matters? Edit: Oh, that error is about Sync not Send I think.

Worth noting that sqlx might be a better choice if you need to use sqlite and async. It's somewhat deliberate that rusqlite is awkward to use in this context, as blocking IO is bad here.

(At least, that's what people more experienced with async tell me is good to do)

[KamilaBorowska]

@broccolihighkicks

This is correct, SQLite is not thread-safe, connections cannot be shared with other threads. Make your connection a thread local or use a mutex.

[malaire]

Worth noting that sqlx might be a better choice if you need to use sqlite and async. It's somewhat deliberate that rusqlite is awkward to use in this context, as blocking IO is bad here.

sqlx is extremely slow with SQLite, up to 500% slower than rusqlite in my tests.

spawn_blocking is also not an option in cases where you need to call async functions during transaction as async is not possible within spawn_blocking.

Currently there doesn't seem to be any good way to use SQLite in async Rust.

[thomcc]

Do you have any suggestions?

[malaire]

I'm attempting to create async-wrapper for rusqlite based on actor which runs inside std::thread::spawn and keeps Connection and Transaction there. Actor can receive closure via mpsc. Closure takes Connection or Transaction as parameter and is run within the thread, returning result via oneshot channel.

This would fix the main problem of sqlx which is really high async overhead, as each query is separate async task: The closure can run any number of queries with fixed async overhead per closure.

This would also allow running async operations during transaction:

1. start transaction
2. run closure against transaction
3. run other async code
4. run other closure against transaction
5. commit

[thomcc]

That sounds cool. If you need something concrete to make stuff easier for that use case, feel free to file issues.

[broccolihighkicks]

https://www.sqlite.org/asyncvfs.html

A async C FFI was added to SQLite, but did not offer much performance gains over using WAL mode.

Maybe it would be OK for your code just to sync wait until the call returns. Assuming the DB file is in WAL mode (so that fsync calls are batched together and the latency cost is only paid once in every 1000 queries or so (every checkpoint)).

[trevyn]

spawn_blocking is also not an option in cases where you need to call async functions during transaction as async is not possible within spawn_blocking.

It doesn't seem like calling async functions from spawn_blocking should be a fundamental problem — basically you just want to call the async function without awaiting it, and then block (block_on?) until the Future succeeds, right? I'm not sure offhand of the best incantation for this, but it seems doable.

[broccolihighkicks]

It doesn't seem like calling async functions from spawn_blocking should be a fundamental problem

But if you have a single threaded event loop there is not any point, as the SQLite function blocks it for the same amount of time - no difference between blocking it now for x or blocking it later for x?

[trevyn]

But if you have a single threaded event loop there is not any point

Right — if you only have one thread, SQLite will block it unless you use an async VFS.

I was considering the case where you push the SQLite transaction to a separate spawn_blocking thread, but you still want to call other async functions from that thread, which is now no longer directly inside an async context itself.

I'm curious what your use case is, though, where you don't have threads available. Is this an embedded thing?

[ekanna]

Async rusqlite built on top of rusqlite.
https://github.com/programatik29/tokio-rusqlite