I want to implement two different structures that have references to each other, can mutate each other, and are thread-safe.
I can introduce the following example:
pub struct Player {
pub x: f32,
pub y: f32,
game: Option<Game>, // Reference to `Game`
}
impl Player {
pub fn new(x: f32, y: f32) -> Self {
Player { x, y, game: None}
}
// Some functions that can change `self`, `game`, and `map`
}
pub struct Game {
pub map: GameMap,
players: Vec<Player>, // Reference to `Player`
}
impl Game {
pub fn new() -> Self {
Game { map: GameMap::new(), players: Vec::new()}
}
pub fn register_player(&mut self, player: Player) {
todo!();
}
}
I'd like to have a similar interface:
fn main() {
let mut p1 = Player::new(0.0, 0.0);
let mut p2 = Player::new(100.0, 100.0);
let mut game = Game::new();
game.register_player(p1);
game.register_player(p2);
p1.forward(); // changes its coordinates using `map` from `game`
p2.shoot(); // changes the `map` and possibly another player
}
I can't use std::rc::Rc and std::cell::RefCell because my goal is to write thread-safety code. I tried std::sync::{Arc, Mutex}, but it's not succeeded yet. How could I solve this challenge?
UPD Here I add the code where I tried to use mutex
use std::sync::{Arc, Mutex};
pub struct Player {
pub x: f32,
pub y: f32,
game: Option<Arc<Mutex<Game>>>,
}
impl Player {
pub fn create(x: f32, y: f32) -> Arc<Mutex<Self>> {
let mut player = Player {
x,
y,
game: None,
};
Arc::new(Mutex::new(player))
}
pub fn mount_game(&mut self, game: Arc<Mutex<Game>>) {
self.game = Some(game);
}
}
pub struct Game {
players: Vec<Arc<Mutex<Player>>>,
}
impl Game {
pub fn create() -> Arc<Mutex<Self>> {
let mut game = Game {
players: Vec::new(),
};
Arc::new(Mutex::new(game))
}
pub fn register_player(&self, game_arc: Arc<Mutex<Self>>, player_arc: Arc<Mutex<Player>>) {
let mut game = game_arc.lock().unwrap();
game.players.push(Arc::clone(&player_arc));
player_arc.lock().unwrap().mount_game(Arc::clone(&game_arc));
}
}
fn main() {
let mut p1 = Player::create(0.0, 0.0);
let mut p2 = Player::create(0.0, 0.0);
let mut game = Game::create();
game.lock().unwrap().register_player(Arc::clone(&game), Arc::clone(&p1));
game.lock().unwrap().register_player(Arc::clone(&game), Arc::clone(&p2));
}
The code you posted deadlocks because you lock the game mutex twice:
main when you call game.lock(),register_player when you call game_arc.lock().Moreover from an API standpoint, needing to pass the game twice to register_player (once as self and once as game_arc) is pretty awkward.
Now if we get rid of the mutex around Game so that we have an Arc<Game>, then we can declare register_player as:
pub fn register_player (self: &Arc<Game>, player_arc: Arc<Mutex<Player>>)
But then we need to put a Mutex inside Game if we want to be able to mutate it:
pub struct Game {
players: Mutex<Vec<Arc<Mutex<Player>>>>,
// Here: ^^^^^
}
In this case Game contains a single field so it's easy, but assuming your game state is more complex, this becomes painful to use.
It would be nice if we could tell the compiler that register_player takes an &Arc<Mutex<Self>>, but unfortunately we can only use types that Deref to Self and Mutex can't (*). We also can't add the register_player method directly on Mutex<Game> like this:
// Not allowed:
impl Mutex<Game> {
pub fn register_player (self: &Arc<Mutex<Game>>, player_arc: Arc<Mutex<Player>>) {
todo!();
}
}
because Mutex is a foreign type. We can however fake it using an extension trait:
pub trait GameExt {
pub fn register_player (self: &Arc<Self>, player_arc: Arc<Mutex<Player>>);
}
impl GameExt for Mutex<Game> {
fn register_player (self: &Arc<Self>, player_arc: Arc<Mutex<Player>>) {
todo!();
}
}
Finally, having an Arc<Player> inside Game and an Arc<Game> inside Player is a bad idea because it risks introducing memory leaks: they will still refer to each other even when you drop all other references, so their respective reference counts will never reach 0 and they will never be freed. This can be avoided by replacing one of the Arcs by a Weak pointer, which will break the cycle.
Full working example (I also removed a bunch of superfluous muts since Arc<Mutex<_>> values don't need to be mut before they're locked):
use std::sync::{Arc, Mutex, Weak};
pub struct Player {
pub x: f32,
pub y: f32,
game: Option<Weak<Mutex<Game>>>,
}
impl Player {
pub fn create (x: f32, y: f32) -> Arc<Mutex<Self>> {
let player = Player {
x,
y,
game: None,
};
Arc::new (Mutex::new (player))
}
pub fn mount_game (&mut self, game: Arc<Mutex<Game>>) {
self.game = Some (Arc::downgrade (&game));
}
pub fn modify_game_state (&self) {
self.game.as_ref().unwrap().upgrade().unwrap().lock().unwrap().state = 42;
}
}
pub struct Game {
state: i32,
players: Vec<Arc<Mutex<Player>>>,
}
impl Game {
pub fn new() -> Arc<Mutex<Self>> {
let game = Game {
state: 0,
players: Vec::new(),
};
Arc::new (Mutex::new (game))
}
}
pub trait GameExt {
fn register_player (self: &Arc<Self>, player_arc: Arc<Mutex<Player>>);
}
impl GameExt for Mutex<Game> {
fn register_player (self: &Arc<Self>, player_arc: Arc<Mutex<Player>>) {
player_arc.lock().unwrap().mount_game (Arc::clone(self));
let mut game = self.lock().unwrap();
game.players.push (player_arc);
}
}
fn main() {
let p1 = Player::create (0.0, 0.0);
let p2 = Player::create (0.0, 0.0);
let game = Game::new();
game.register_player (p1); // or Arc::clone (&p1) if you need to use p1 later in this function
game.register_player (Arc::clone (&p2));
p2.lock().unwrap().modify_game_state();
}
(*) Mutex<Foo> can't Deref to Foo because it needs somewhere to put the MutexGuard while the reference is live, and Deref doesn't offer anywhere to put the guard.