Compiler says that data cannot be shared between threads safely even though the data is wrapped within a Mutex

Question

I'm using Rocket which has a State that it passes to the HTTP requests. This struct contains a Mutex<DatastoreInstance> which gives access to a SQLite database and is locked with a mutex to make read and writes safe.

pub struct DatastoreInstance {
    conn: Connection,
}

When the DatastoreInstance struct looked like this, with only a SQLite connection everything worked fine, but I then also wanted to add a transaction object within this struct:

pub struct DatastoreInstance {
    conn: Connection,
    events_transaction: Transaction,
}

This did not compile because the Transaction object needs to reference a Connection object which should have a lifetime which it is aware of. The Connection and Transaction objects within rusqlite which I am using are defined as following:

pub struct Connection {
    db: RefCell<InnerConnection>,
    cache: StatementCache,
    path: Option<PathBuf>,
}

pub struct Transaction<'conn> {
    conn: &'conn Connection,
    drop_behavior: DropBehavior,
}

To solve the lifetime issues I had to add these lifetime parameters to get it working:

pub struct DatastoreInstance<'a> {
    conn: Connection,
    events_transaction: Transaction<'a>,
}

This was the result and was supposed to work according to my understanding of both lifetimes and mutexes, but I now get a compiler error telling me:

`std::cell::RefCell<lru_cache::LruCache<std::string::String, rusqlite::raw_statement::RawStatement>>` cannot be shared between threads safely
    |                                                                                                            
    = help: within `rusqlite::Connection`, the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<lru_cache::LruCache<std::string::String, rusqlite::raw_statement::RawStatement>>`
    = note: required because it appears within the type `rusqlite::cache::StatementCache`                        
    = 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 `datastore::DatastoreInstance<'_>`                       
    = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Mutex<datastore::DatastoreInstance<'_>>`
    = note: required because it appears within the type `endpoints::ServerState<'_>`                             
    = note: required by `rocket::State`

According to my understanding of mutexes, this code should be valid because the whole DatastoreInstance struct is wrapped within a Mutex which should guarantee that only one thread is referencing this object at a time.

What am I missing?

Why doesn't the compiler find RefCell to be safe anymore after being within a Connection referenced within a Transaction instead of solely within a Connection?

Do I have a bad understanding of how mutexes work? Are my lifetimes invalid and somehow break read/write safety? Is the design of having the Connection and Transaction within the same struct a bad design which breaks read/write safety? Do I need to redesign my data structures somehow to make this safe? Or am I just missing something very obvious?

Answer 1

A Mutex is only Send or Sync if the value it contains is itself Send:

impl<T: ?Sized + Send> Send for Mutex<T>    
impl<T: ?Sized + Send> Sync for Mutex<T>

A &T is only Send when T is Sync:

impl<'a, T> Send for &'a T
where
    T: Sync + ?Sized, 

And a RefCell is never Sync

impl<T> !Sync for RefCell<T>
where
    T: ?Sized, 

As the error message states, your transaction contains a reference to a RefCell. It doesn't matter that there's a mutex, it's inherently not memory-safe to share it across threads. A simple reproduction:

use std::{cell::RefCell, sync::Mutex};

struct Connection(RefCell<i32>);
struct Transaction<'a>(&'a Connection);

fn is_send<T: Send>(_: T) {}

fn main() {
    let c = Connection(RefCell::new(42));
    let t = Transaction(&c);
    let m = Mutex::new(t);

    is_send(m);
}

error[E0277]: `std::cell::RefCell<i32>` cannot be shared between threads safely
  --> src/main.rs:13:5
   |
13 |     is_send(m);
   |     ^^^^^^^ `std::cell::RefCell<i32>` cannot be shared between threads safely
   |
   = help: within `Connection`, the trait `std::marker::Sync` is not implemented for `std::cell::RefCell<i32>`
   = note: required because it appears within the type `Connection`
   = note: required because of the requirements on the impl of `std::marker::Send` for `&Connection`
   = note: required because it appears within the type `Transaction<'_>`
   = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Mutex<Transaction<'_>>`
note: required by `is_send`
  --> src/main.rs:6:1
   |
6  | fn is_send<T: Send>(_: T) {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^

Why doesn't the compiler find RefCell to be safe anymore after being within a Connection referenced within a Transaction instead of solely within a Connection?

The RefCell is fine, it's the reference to a RefCell that is not.

Is the design of having the Connection and Transaction within the same struct a bad design [...] Do I need to redesign my data structures

Yes.

• How to store rusqlite Connection and Statement objects in the same struct in Rust?
• Why can't I store a value and a reference to that value in the same struct?