Share Connection into several threads about rusqlite HOT 23 OPEN

Yet, tests/deny_single_threaded_sqlite_config.rs checks sqlite3 has been compiled with multithread enabled, in the goal to share connections through threads.

No, if you try to share a single Connection across more than one thread, you will get the following error (I was wondering about this as well):

  --> src/sqlite.rs:68:13
   |
12 |     let x = ::std::thread::spawn(|| {
   |             ^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<rusqlite::InnerConnection>`
   |
   = note: `std::cell::RefCell<rusqlite::InnerConnection>` cannot be shared between threads safely
   = note: required because it appears within the type `rusqlite::Connection`
   = note: required because it appears within the type `&rusqlite::Connection`
   = note: required because of the requirements on the impl of `std::marker::Send` for `&&rusqlite::Connection`
   = note: required because it appears within the type `[closure@src/sqlite.rs:68:34: 71:6 y:&&rusqlite::Connection]`
   = note: required by `std::thread::spawn`

Using the following code:

let mut conn = Connection::open_in_memory().unwrap();
let conn_ref = &conn;
let handle = ::std::thread::spawn(|| {
    conn_ref.execute("SELECT * FROM XY", &[]);
});
handle.join();

The inline documentation for creating an InnerConnection states the following (Source Documentation):

// Before opening the database, we need to check that SQLite hasn't been
// compiled or configured to be in single-threaded mode. If it has, we're
// exposing a very unsafe API to Rust, so refuse to open connections at all.

I think this is just used as a precaution. If there exists another handle to the sqlite database outside of your application, things could get messy if the database is in Single-thread mode Using SQLite In Multi-Threaded Applications. Nonetheless, a short passage in the README would be nice.

from rusqlite.

There are actually three distinct levels of thread safety possible with SQLite.

Depending on how SQLite was compiled (and whether rusqlite explicitly specifies SQLITE_OPEN_NOMUTEX vs SQLITE_OPEN_FULLMUTEX), rusqlite's behavior here could be viewed as overly strict. (There are obvious performance ramifications to using FULLMUTEX serialized mode).

from rusqlite.

slight newbie question, but the Send but not Sync behavior leads to some pretty confusing behavior around async code because when a task wakes up we basically need to reconnect to the database between every await?

Unless I'm missing a code paradigm...

from rusqlite.

I'm going to reopen to collect feedback (and so I can more easily find the issue), but I expect that the resolution here will probably be documentation.

from rusqlite.

But if I understand correctly, this means concurrency IS increased if the same process just makes multiple connections?

Yes.

So maybe it's an idea to offer a rusqlite-pool crate or something that has multiple connections and hands them out?

I think https://crates.io/crates/r2d2_sqlite is popular, based on https://crates.io/crates/r2d2.

from rusqlite.

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

from rusqlite.

Currently,
the Rust type system ensures that one rusqlite::Connection can be used by only one thread at the same time because it is not Synced.
But rusqlite::Connection is Sendable so rusqlite must ensure that the SQLite library is thread-safe (i.e. not in single-thread mode).

The SQLITE_OPEN_NOMUTEX flag is set to pass this information to SQLite:
http://sqlite.org/c3ref/open.html:

If the SQLITE_OPEN_NOMUTEX flag is set, then the database connection opens in the multi-thread threading mode as long as the single-thread mode has not been set at compile-time or start-time.

http://sqlite.org/threadsafe.html:

Multi-thread. In this mode, SQLite can be safely used by multiple threads provided that no single database connection is used simultaneously in two or more threads.

from rusqlite.

@gwenn is completely correct. To add a little additional info on the quoted bit above about checking for single-threaded SQLite - it's not just a precaution. Connections can be sent between threads as gwenn mentioned. The more general case is that it's also fine to have multiple distinct Connections in use on different threads simultaneously. That is the one I meant by the comment about "exposing a very unsafe API to Rust", since that would explode spectacularly at the SQLite level if it was compiled for single-thread use only.

I'm not sure exactly what to add to the README. One of the main goals of rusqlite is that the library takes care of this kind of concern for you, and the compiler refuses to allow you to do things that might be unsafe (such as both the lazy_static and the attempt to share a connection between threads).

from rusqlite.

Overly strict should be:
not Syncable (Rust default if there is a *mut somewhere) + SQLITE_OPEN_FULLMUTEX (SQLite default).

from rusqlite.

it's not just a precaution. Connections can be sent between threads as gwenn mentioned.

Which doesn't matter at all because the access to the conn must still be synchronised -- at certain point you'd like to protect against the conn becoming a pointer to meaningless data (which happens after close).

So IMO, and since sqlite3 is working on serialized mode by default on most platforms, enabling the conn be wrappable inside a RwLock would make sense: no actual locking on the Rust side (only on sqlite side) when reading the conn handle, and exclusive lock when writing (close).

Am I missing something?

from rusqlite.

slight newbie question, but the Send but not Sync behavior leads to some pretty confusing behavior around async code because when a task wakes up we basically need to reconnect to the database between every await?

Unless I'm missing a code paradigm...

I think it's incorrect to call SQLite directly from an async context, because it can "block" in the async ecosystem sense: run for a significant amount of time, wait for IO, etc.

Instead, it seems to be correct to use your executor's spawn_blocking function or equivalent to get a blocking context from your executor's "intended for blocking" thread pool, which can then be glued into an async context, e.g.:

async fn my_async_fn() {
  tokio::task::spawn_blocking(|| {
    // ... blocking stuff here, including SQLite calls ...
  }).await?;
}

If you just need a few SQLite calls in sequence, you can do them all inside that one blocking closure. If it's something more complicated, the answer is more complicated, but you should never need to open multiple rusqlite::Connections just because of async.

from rusqlite.

@trevyn I'm aware of how spawn_blocking works, but the issue here is more around being able to store Connection instances into the Future's suspended state, which lacking Sync I think means it cannot be done?

from rusqlite.

That sounds correct — I think the Connection instance should be stored in the spawn_blocking closure, implicitly (but not directly) in the Future's suspended state. This implies that the closure should span the range of code that needs access to that Connection.

If you need to call "back into" async, I think this can be done with block_on or similar.

Ensuring that Connection is not Sync seems to be an important implication of the SQLite API design, see #342 (comment).

So Connection is tied to a single thread, but you can create a closure that represents that single thread, which is then also able to call back into the async ecosystem as necessary.

from rusqlite.

Might also be possible to put the Connection in a Mutex and keep that in scope that across .awaits, as in #697 (comment). That seems more awkward to me, but maybe not for other use cases.

Edit: Forgot that "holding" a mutex refers to keeping it locked... I meant just keeping it in scope.

from rusqlite.

Wait, so could Connection have an internal Mutex that does this for you? Not sure that's always the best tradeoff, but maybe useful as an option.

from rusqlite.

Might also be possible to put the Connection in a Mutex and hold that across .awaits

MutexGuard in the stdlib is not Send, so this would have the same issue. I gather this is also considered an antipattern for the same reason. You could use tokio's async Mutex or whatever though.

I think adding support for handling an async mutex inside rusqlite wouldn't be worth it.

from rusqlite.

MutexGuard in the stdlib is not Send, so this would have the same issue.

Right, I was thinking of just keeping the Mutex in scope across .await and repeatedly unlocking it as necessary. (As is common any other time you want to share a Connection across threads.)

I agree that such a mechanism is probably not worth adding to rusqlite itself. That said, I do think it's worth collecting and documenting approaches to this problem, because it seems to be quite vexing and reasonably widespread. :)

from rusqlite.

@thomcc , I think it might be useful to maybe have a feature to be able to signal that you have built (or want to build if bundled), the SQLite lib with Serialized mode for multithreading.
An example of how this could be useful inside the embedded server we have, is that Reads often just use prepare (&self) and when writing you use a transaction (which requires &mut self), I would suspect that I would be able to use a RwLock here, but it's not possible because of the !Sync, meaning that reads also block each other ( which is unnecessary when used with the serialized mode)

from rusqlite.

Serialized mode just moves the locking into the sqlite library no? So unless I'm mistaken, this wouldn't increase concurrency at all, it would just move the locking into sqlite's code instead of in Rust.

from rusqlite.

Serialized mode just moves the locking into the sqlite library no? So unless I'm mistaken, this wouldn't increase concurrency at all, it would just move the locking into sqlite's code instead of in Rust.

I don't know at what level the locking would occur in sqlite, if it maybe does it per table for example, than there's a potential point where concurrency would improve. Also does the prepatory work (i.e. making the prepared statement but not yet executing) occur any kind of locking in the serialized mode, because that's another point where having multiple threads being able to do that could improve concurrency.

I don't seem to find any details about it in the docs, so I'm not sure where the mutexes get used, so it's difficult to tell.

from rusqlite.

The per-connection locking is different than the database locking itself (described here or here for WAL mode). It's plausible that a future version of SQLite could do per-table locking for the database locking protocol (probably far future — it would be a huge change), but for operations on the connection it would be very hard to see how that could work.

SQLite currently also takes the lock during prepare: https://github.com/sqlite/sqlite/blob/master/src/prepare.c#L654

I don't really see how it could avoid it — there's too much per-connection stuff that needs to be considered.

from rusqlite.

@thomcc , in that case it indeed doesn't give much(/any?) benefit. But this should probably be well documented as to why this is.

But if I understand correctly, this means concurrency IS increased if the same process just makes multiple connections? So maybe it's an idea to offer a rusqlite-pool crate or something that has multiple connections and hands them out? I certainly don't think it falls into the scope of rusqlite, but it might be nice to have something like this under the rusqlite organization on github? Than the documentation here can just point to that crate.

EDIT: I'm not trying to say that you have to spend resources and time to make that secondary crate, but I meant more in the way of: are you open to have such a crate in the github organization?

from rusqlite.

rust newbie here. I'm following the Actix web tutorial to connect to database. Their example https://github.com/actix/examples/blob/master/databases/sqlite/src/main.rs is indeed using r2d2 SqliteConnectionManager.

Their example feels minimal and correct but I don't have enough experience to judge whether that's the best solution. Can people help assess if I should treat that example as canonical?

from rusqlite.