Second mutable borrow occurs here ! I want your opinion please

Hello,

I've just come across a case where I need to make a modification to a Hashmap as well as a deletion.

here's just the part of the code that causes this error :

lazy_static! {
    static ref OPENED_BRIDGES: RwLock<HashMap<u32, Arc<Mutex<BridgeConnection>>>> =
        RwLock::new(HashMap::new());
}

#[derive(Debug)]
struct BridgeConnection {
    address: String,
    port: u32,
    writer: Arc<Mutex<WriteHalf<TcpStream>>>,
}

async fn remove_bridge(id_bridge: u32) -> bool {
    let mut bridges = OPENED_BRIDGES.write().await;

    if let Some(bridge) = bridges.get_mut(&id_bridge) {

        let bridge = bridge.lock().await;
        if bridge.writer.lock().await.shutdown().await.is_err() {
                false;
        }

        bridges.remove(&id_bridge).is_some()
    } else {
        true
    }
}
error[E0499]: cannot borrow `bridges` as mutable more than once at a time
   --> src/lib.rs:114:9
    |
110 |     if let Some(bridge) = bridges.get_mut(&id_bridge) {
    |                           ------- first mutable borrow occurs here
...
114 |         bridges.remove(&id_bridge).is_some()
    |         ^^^^^^^ second mutable borrow occurs here
115 |     } else {
    |     - first borrow might be used here, when `bridge` is dropped and runs the `Drop` code for type `tokio::sync::MutexGuard`

I've already solved the problem, but I didn't like the way I solved it, by creating a sub-Scope, like this :
(and the borrow checker error doesn't help at all, if I had followed it I would never have been able to fix the error.)

async fn remove_bridge(id_bridge: u32) -> bool {
       let mut bridges = OPENED_BRIDGES.write().await;

       if let Some(bridge) = bridges.get_mut(&id_bridge) {
        { // <-------------------------- Solution START
            let bridge = bridge.lock().await;
            if bridge.writer.lock().await.shutdown().await.is_err() {
                false;
            }
        } // <-------------------------- Solution END
        bridges.remove(&id_bridge).is_some()
    } else {
        true
    }
}

I wonder if there's a cleaner way to do it, and also if the borrow checker can't be a little more inteligent to understand that the action is safe?

Thank you in advance for your opinions.

Explicitly drop your lock when you're done with it.

async fn remove_bridge(id_bridge: u32) -> bool {
    let mut bridges = OPENED_BRIDGES.write().await;

    if let Some(bridge) = bridges.get_mut(&id_bridge) {
        let bridge = bridge.lock().await;
        bridge.writer.lock().await.shutdown().await.unwrap();
        drop(bridge);
        bridges.remove(&id_bridge).is_some()
    } else {
        true
    }
}

The inner lock references an item you're trying to remove (move), so it would be unsound to allow it to exist after the removal. Moving something is an exclusive action.

Maybe what you really wish for is different drop semantics.

2 Likes

Thanks for your suggestion, with the Drop it's cleaner than the scope.

But don't you think that the error message isn't pointing to the right zone that's causing the problem?

I agree the error message could be more helpful, ideally suggesting the explicit drop. It does identify the drop of bridge to be the problem, but it's also true that it is the last thing mentioned, after highlighting bridges a couple times. I.e. at least in this case, the part you need to change to fix the problem is not emphasized enough.

If you can mock up what you wish it had said, you might want to file a diagnostic issue. I didn't find and existing one for this particular case (but maybe that's just bad search-fu).

1 Like

It's a good thing the portion of code in this function was small. If the code were much longer and more complicated, resolving an error of this kind would be very difficult.

I'll try to create a simple code and put it on the Playground, if it can help the Rust development team to improve the diagnosis of this kind of cases.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.