Forced to use of Mutex when it's not required

Question

I am writing a game and have a player list defined as follows:

pub struct PlayerList {
    by_name: HashMap<String, Arc<Mutex<Player>>>,
    by_uuid: HashMap<Uuid, Arc<Mutex<Player>>>,
}

This struct has methods for adding, removing, getting players, and getting the player count.

The NetworkServer and Server shares this list as follows:

NetworkServer {
    ...
    player_list: Arc<Mutex<PlayerList>>,
    ...
}

Server {
    ...
    player_list: Arc<Mutex<PlayerList>>,
    ...
}

This is inside an Arc<Mutex> because the NetworkServer accesses the list in a different thread (network loop).
When a player joins, a thread is spawned for them and they are added to the player_list.

Although the only operation I'm doing is adding to player_list, I'm forced to use Arc<Mutex<Player>> instead of the more natural Rc<RefCell<Player>> in the HashMaps because Mutex<PlayerList> requires it. I am not accessing players from the network thread (or any other thread) so it makes no sense to put them under a Mutex. Only the HashMaps need to be locked, which I am doing using Mutex<PlayerList>. But Rust is pedantic and wants to protect against all misuses.

As I'm only accessing Players in the main thread, locking every time to do that is both annoying and less performant. Is there a workaround instead of using unsafe or something?

Here's an example:

use std::cell::Cell;
use std::collections::HashMap;
use std::ffi::CString;
use std::rc::Rc;
use std::sync::{Arc, Mutex};
use std::thread;

#[derive(Clone, Copy, PartialEq, Eq, Hash)]
struct Uuid([u8; 16]);

struct Player {
    pub name: String,
    pub uuid: Uuid,
}

struct PlayerList {
    by_name: HashMap<String, Arc<Mutex<Player>>>,
    by_uuid: HashMap<Uuid, Arc<Mutex<Player>>>,
}

impl PlayerList {
    fn add_player(&mut self, p: Player) {
        let name = p.name.clone();
        let uuid = p.uuid;

        let p = Arc::new(Mutex::new(p));
        self.by_name.insert(name, Arc::clone(&p));
        self.by_uuid.insert(uuid, p);
    }
}

struct NetworkServer {
    player_list: Arc<Mutex<PlayerList>>,
}

impl NetworkServer {
    fn start(&mut self) {
        let player_list = Arc::clone(&self.player_list);
        thread::spawn(move || {
            loop {
                // fake network loop
                // listen for incoming connections, accept player and add them to player_list.
                player_list.lock().unwrap().add_player(Player {
                    name: "blahblah".into(),
                    uuid: Uuid([0; 16]),
                });
            }
        });
    }
}

struct Server {
    player_list: Arc<Mutex<PlayerList>>,
    network_server: NetworkServer,
}

impl Server {
    fn start(&mut self) {
        self.network_server.start();
        // main game loop
        loop {
            // I am only accessing players in this loop in this thread. (main thread)
            // so Mutex for individual player is not needed although rust requires it.
        }
    }
}

fn main() {
    let player_list = Arc::new(Mutex::new(PlayerList {
        by_name: HashMap::new(),
        by_uuid: HashMap::new(),
    }));
    let network_server = NetworkServer {
        player_list: Arc::clone(&player_list),
    };
    let mut server = Server {
        player_list,
        network_server,
    };
    server.start();
}

Answer 1

As I'm only accessing Players in the main thread, locking everytime to do that is both annoying and less performant.

You mean, as right now you are only accessing Players in the main thread, but at any time later you may accidentally introduce an access to them in another thread?

From the point of view of the language, if you can get a reference to a value, you may use the value. Therefore, if multiple threads have a reference to a value, this value should be safe to use from multiple threads. There is no way to enforce, at compile-time, that a particular value, although accessible, is actually never used.

This raises the question, however:

If the value is never used by a given thread, why does this thread have access to it in the first place?

It seems to me that you have a design issue. If you can manage to redesign your program so that only the main thread has access to the PlayerList, then you will immediately be able to use Rc<RefCell<...>>.

For example, you could instead have the network thread send a message to the main thread announcing that a new player connected.

At the moment, you are "Communicating by Sharing", and you could shift toward "Sharing by Communicating" instead. The former usually has synchronization primitives (such as mutexes, atomics, ...) all over the place, and may face contention/dead-lock issues, while the latter usually has communication queues (channels) and requires an "asynchronous" style of programming.

Answer 2

I suggest solving this threading problem using a multi-sender-single-receiver channel. The network threads get a Sender<Player> and no direct access to the player list.

The Receiver<Player> gets stored inside the PlayerList. The only thread accessing the PlayerList is the main thread, so you can remove the Mutex around it. Instead in the place where the main-thread used to lock the mutexit dequeue all pending players from the Receiver<Player>, wraps them in an Rc<RefCell<>> and adds them to the appropriate collections.

---

Though looking at the bigger designing, I wouldn't use a per-player thread in the first place. Instead I'd use some kind single threaded event-loop based design. (I didn't look into which Rust libraries are good in that area, but tokio seems popular)

Answer 3

Send is a marker trait that governs which objects can have ownership transferred across thread boundaries. It is automatically implemented for any type that is entirely composed of Send types. It is also an unsafe trait because manually implementing this trait can cause the compiler to not enforce the concurrency safety that we love about Rust.

The problem is that Rc<RefCell<Player>> isn't Send and thus your PlayerList isn't Send and thus can't be sent to another thread, even when wrapped in an Arc<Mutex<>>. The unsafe workaround would be to unsafe impl Send for your PlayerList struct.

Putting this code into your playground example allows it to compile the same way as the original with Arc<Mutex<Player>>

struct PlayerList {
    by_name: HashMap<String, Rc<RefCell<Player>>>,
    by_uuid: HashMap<Uuid, Rc<RefCell<Player>>>,
}

unsafe impl Send for PlayerList {}

impl PlayerList {
    fn add_player(&mut self, p: Player) {
        let name = p.name.clone();
        let uuid = p.uuid;

        let p = Rc::new(RefCell::new(p));
        self.by_name.insert(name, Rc::clone(&p));
        self.by_uuid.insert(uuid, p);
    }
}

Playground

The Nomicon is sadly a little sparse at explaining what rules have have to be enforced by the programmer when unsafely implementing Send for a type containing Rcs, but accessing in only one thread seems safe enough...

For completeness, here's TRPL's bit on Send and Sync