Diesel: Connection probably needs to be Send + Sync

Created on 4 Feb 2016 · 4Comments · Source: diesel-rs/diesel

I'm working on updating r2d2-diesel for Connection's change from a struct to a trait (sgrif/r2d2-diesel#3) and running into this error:

tests/test.rs:14:18: 14:49 error: the trait `core::marker::Sync` is not implemented for the type `alloc::rc::Rc<diesel::pg::connection::raw::RawConnection>` [E0277]
tests/test.rs:14     let manager: ConnectionManager<PgConnection> = ConnectionManager::new("postgres://localhost");
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
tests/test.rs:14:18: 14:49 help: run `rustc --explain E0277` to see a detailed explanation
tests/test.rs:14:18: 14:49 note: `alloc::rc::Rc<diesel::pg::connection::raw::RawConnection>` cannot be shared between threads safely
tests/test.rs:14:18: 14:49 note: required because it appears within the type `diesel::pg::connection::PgConnection`
tests/test.rs:14:18: 14:49 note: required by `r2d2_diesel::ConnectionManager`

This is happening because r2d2's ManageConnection trait is Send + Sync + 'static.

This is what PgConnection looks like right now:

pub struct PgConnection {
    raw_connection: Rc<RawConnection>,
    transaction_depth: Cell<i32>,
}

Any reason not to change Rc to Arc and impl Sync for PgConnection? If not, can Send + Sync be added as constraints for Connection, or are there cases where a connection would be restricted to one thread?

Source

jimmycuadra

Most helpful comment

Connections are very much only intended to be used on a single thread. This isn't just true of Diesel, but DBs in general. We do implement Send. I'm unsure why the Sync constraint exists on r2d2, but it shouldn't have it. You'll probably need to wrap it in a Mutex to make it happy. :\

The reasoning for using Rc instead of Arc there is covered in the commit that added it

sgrif on 4 Feb 2016

👍3

All 4 comments

The reasoning for using Rc instead of Arc there is covered in the commit that added it

sgrif on 4 Feb 2016

👍3

The SqliteConnection doesn't implement Send. I dont think this restriction is useful, since Sqlite can be (and usually is) compiled with multi-thread support (And PgConnection implements Send).

rusqlite uses unsafe impl Send for Connection {} to archieve that.