Connection probably needs to be Send + Sync

[jimmycuadra]

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?

[sgrif]

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

[krypt-n]

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.