Search code examples
rustborrow-checkerrust-tokio

How to restructure struct such that 'Cannot borrow `x` as mutable more than once at a time' does not occur


Currently, I have one AppState struct that has two fields:

pub struct AppState {
    players: HashMap<String, PlayerValue>,
    cars: HashMap<String, CarValue>,
}

I have implemented a multi-threaded server application using tokio that processes requests and responds to them by mutating the fields in AppState.

For instance:

// Process stop request
let mut app_state = app_state_clone.lock().await; // `app_state_clone` is  `Arc<Mutex<AppState>>`
app_state.stop_players().await?;
app_state.stop_cars().await?;
Ok(())

This code compiles and works as expected. However, in this case we first wait on the completion of the app_state.stop_players() future, and then again wait on the completion of app_state.stop_cars().

However, since both methods could point to different parts of the structure and do not mutate the same fields, I thought I could just use try_join! to wait concurrently on the tasks.

// Process stop request
let mut app_state = app_state_clone.lock().await; // `app_state_clone` is  `Arc<Mutex<AppState>>`
let stop_players_f = app_state.stop_players();
let stop_cars_f = app_state.stop_cars();
try_join!(stop_players_f, stop_cars_f);
Ok(())

This will lead to the compile error:

cannot borrow app_state as mutable more than once at a time

I have searched for ways to restructure my code to fix this and found the following SO answer:

let mut x = X { a: 1, b: 2 };
let a = &mut x.a;
let b = &mut x.b;

Here the compiler can see that a and b never point to the same data, even though they do point inside of the same structure.

Inspired by this, I thought I can restructure my code as follows:

pub struct AppState {
    players_state: PlayersState,
    cars_state: CarsState,
}

pub struct PlayersState {
    players: HashMap<String, PlayerValue>,
}

pub struct CarsState {
    cars: HashMap<String, CarValue>,
}

Code in the server method:

// Process stop request
let players_state = &mut app_state.players_state;
let cars_state = &mut app_state.cars_state;
let stop_players_f = players_state.stop_players();
let stop_cars_f = cars_state.stop_cars();
try_join!(stop_players_f, stop_cars_f);
Ok(())

However, this will just lead to the same error:

cannot borrow app_state as mutable more than once at a time

EDIT: Here is the full compile error:

error[E0499]: cannot borrow `app_state` as mutable more than once at a time
    --> crates/my-app/src/app.rs:1786:68
     |
1785 | ...                   let players_state = &mut app_state.players_state;
     |                                                --------- first mutable borrow occurs here
1786 | ...                    let cars_state = &mut app_state.cars_state;
     |                                              ^^^^^^^^^ second mutable borrow occurs here
...
1791 | ...                    let stop_players_f = players_state.stop_players();
     |                                              --------------------------- first borrow later used here

And here is the implementation of PlayersState:

impl PlayersState {
    pub fn players(&self) -> &HashMap<String, PlayerValue> {
        &self.players
    }

    pub fn players_mut(&mut self) -> &mut HashMap<String, PlayerValue> {
        &mut self.players
    }

    pub async fn stop_players(&self) -> Result<(), StopPlayersError> {
        for player in self.players.values() {
            match player {
                PlayerValue::MyOnePlayer(p) => {
                    p.stop().await?;
                }
            }
        }
        Ok(())
    }
}

Note: While mut in stop_players is not required, it is necessary in the stop_cars function.

More insights into this problem would be greatly appreciated, since I do not seem to understand how I can solve this.

EDIT:

The following code represents an actual minimal example that reproduces the error:

use std::collections::HashMap;
use tokio::try_join;
use tokio::sync::Mutex;
use std::sync::Arc;

pub struct App {
    state: Arc<Mutex<AppState>>,
}

pub struct AppState {
    players_state: PlayersState,
    cars_state: CarsState,
}

pub enum PlayerValue {
    MyOnePlayer(PlayerInner)
}

pub struct PlayerInner;

impl PlayerInner {
    async fn stop(&self) -> Result<(), ()> { Ok(()) }
}

pub struct PlayersState {
    players: HashMap<String, PlayerValue>,
}

impl PlayersState {
    pub async fn stop_players(&self) -> Result<(), ()> {
        for player in self.players.values() {
            match player {
                PlayerValue::MyOnePlayer(p) => {
                    p.stop().await?;
                }
            }
        }
        Ok(())
    }
}

pub struct CarsState {
    cars: HashMap<String, ()>,
}

impl CarsState {
    async fn stop_cars(&mut self) -> Result<(), ()> { Ok(()) }
}

pub async fn check() -> Result<(), ()> {

    // Init on app startup 

    let state =  Arc::new(Mutex::new(AppState {
        players_state: PlayersState {
            players: vec![].into_iter().collect()
        },
        cars_state: CarsState {
            cars: vec![].into_iter().collect()
        },
    }));
    
    
    let app = App {
        state
    };
    
    
    // This code will be executed when we process a request
    // `app.state` is in the real 'code' a clone, because I have to use it in the request/response loop and UI loop
    
    let mut app_state = app.state.lock().await;
    
    let players_state = &mut app_state.players_state;
    let cars_state = &mut app_state.cars_state;
    let stop_players_f = players_state.stop_players();
    let stop_cars_f = cars_state.stop_cars();
    try_join!(stop_players_f, stop_cars_f);
    Ok(())
}

Solution

  • Minimized example:

    use std::sync::Mutex;
    
    pub struct AppState {
        players_state: (),
        cars_state: (),
    }
    
    pub fn check() {
        let state = Mutex::new(AppState {
            players_state: (),
            cars_state: (),
        });
    
        let mut app_state = state.lock().unwrap();
        let players_state = &mut app_state.players_state;
        let _cars_state = &mut app_state.cars_state;
        println!("{:?}", players_state);   
    }
    

    Playground
    Here we're using the standard sync Mutex, but the error will be essentially the same no matter what synchronization primitive one uses.


    The reason for this error is the fact that accessing properties on app_state must come through the DerefMut implementation of MutexGuard. In other words, the problematic part is in fact desugared to something like this:

    let players_state = &mut DerefMut::deref_mut(&mut app_state).players_state;
    let _cars_state = &mut DerefMut::deref_mut(&mut app_state).cars_state;
    

    And DerefMut::deref_mut is not special when it comes to borrow-checking - it takes &mut self, so it assumes that implementation can access any field in any way, therefore invalidating any previously existing reference to the struct.

    Linked answer, however, didn't suffer from this problem, because there we had &mut X directly - in this case compiler is able to reason about disjointness of borrows and allow references to different fields to coexist.


    Therefore, to achieve the same result, you have to somehow convert MutexGuard<AppState> to &mut AppState before borrowing its fields. Luckily, if we don't cross the function boundary (i.e. don't try to return anything to the caller), that's quite easy, and the very code above hints on the way to do: one can simply extract the common part of desugared code:

    let app_state: &mut AppState = DerefMut::deref_mut(&mut app_state);
    let players_state = &mut app_state.players_state;
    let _cars_state = &mut app_state.cars_state;
    

    And, since deref_mut is an implementation of dereference operation, that can be simplified to this:

    let app_state = &mut *app_state;
    let players_state = &mut app_state.players_state;
    let _cars_state = &mut app_state.cars_state;
    

    With this change, example compiles - playground.