Very Un-Rust like way of working with CondVar

1

Manually dropping MutexGuard is simply bad design in my opinion and very similar to manual memory management in C...

Any thoughts? Anyone?

2

Let's look at the problem for another PoV. What would be the desired behavior ? How would the compiler know when to release the lock ?

3 Likes
3

Well, not quite.

Rust will eventually clean up all resources. The problem with locks is that it really matters when exactly the clean up happens.

The rules in which order and when the clean up is executed are somewhat complex. In the section on drop scopes the most important rule is found:

Each variable or temporary is associated to a drop scope. When control flow leaves a drop scope all variables associated to that scope are dropped in reverse order of declaration (for variables) or creation (for temporaries).

[…]

Given a function, or closure, there are drop scopes for:

  • The entire function
  • Each statement
  • Each expression
  • Each block, including the function body
    • In the case of a block expression, the scope for the block and the expression are the same scope.
  • Each arm of a match expression

In the referenced example, you create a variable which gets dropped at the end of the block.

Even more confusing can be temporary scopes:

The temporary scope of an expression is the scope that is used for the temporary variable that holds the result of that expression when used in a place context, unless it is promoted.

Apart from lifetime extension, the temporary scope of an expression is the smallest scope that contains the expression and is one of the following:

  • The entire function body.
  • A statement.
  • The body of a if, while or loop expression.
  • The else block of an if expression.
  • The condition expression of an if or while expression, or a match guard.
  • The expression for a match arm.
  • The second operand of a lazy boolean expression.

This can be in particular confusing because if and match behave differently:

struct Resource;

impl Resource {
    fn new() -> Self {
        println!("Creating `Resource`");
        Self
    }
    fn check(&self) -> bool {
        true
    }
}

impl Drop for Resource {
    fn drop(&mut self) {
        println!("Cleaning up `Resource`");
    }
}

fn main() {
    {
        println!("Using a variable:");
        let r = Resource::new();
        if r.check() {
            println!("Method `check` returned true");
        }
    }
    println!("");
    {
        println!("Using `if`:");
        if Resource::new().check() {
            println!("Method `check` returned true");
        }
    }
    println!("");
    {
        println!("Using `match`:");
        match Resource::new().check() {
            true => println!("Method `check` returned true."),
            _ => (),
        }
    }
}

(Playground)

Output:

Using a variable:
Creating `Resource`
Method `check` returned true
Cleaning up `Resource`

Using `if`:
Creating `Resource`
Cleaning up `Resource`
Method `check` returned true

Using `match`:
Creating `Resource`
Method `check` returned true.
Cleaning up `Resource`

The tricky part with MutexGuards (or other lock guards) is that it often matters to release them quickly. But that's not always true. The compiler can't know how long you really need to hold the lock, so you have to tell the compiler.

If you don't manually specify how long the lock will be held, then Rust will (usually) fall back to clean up the resources at the end of the block, i.e. hold the lock until the block ends. (An exception is the temporary scope in the if expression as demonstrated above.)

No, because in C, if you forget to clean up a resource, it gets never cleaned up and will leak. Leaking is very difficult to achive accidentally in Rust.

However, causing deadlocks is easy in Rust.

3 Likes
4

This

5

Surely, logical behaviour would be before the call to notify_*? Just like in call wait it waits on MutexGuard.

6

One problem is that CondVar has an unergonomic interface. The compiler doesn't know that a certain Mutex is only used with a particular CondVar.

7

Same reasoning like condvar.wait(guard). The compiler couldn't know, yet condvar waits on that particular mutexguard. So it should release it.

8

Ok, but how would the compiler know that some method (in that case the destructor) is to be called at exactly that point ? Why not before tx.send() or println!() or any other method ?

The mutex(guard)/condvar are regular structs, there is nothing special about them. So why would the compiler do anything special with them ? It just follows the RAII rules.

3 Likes
9

Same reasoning like condvar.wait(guard). The compiler couldn't know, yet condvar waits on that particular mutexguard. So it should release it.

10

Can you try to explain (using a code example) when the compiler exactly should release a guard?

2 Likes
11

That's almost always the wrong position to release the lock - you usually want to hold the lock until the end of the call to notify_* (if not long after that point) so that there's no race window between releasing the lock, and the associated wait re-locking the condition variable.

A common pattern in code I've written that uses condition variables is to take the lock, and then alternate mutation of the data structure the lock protects with notification of condition variables. I usually still want to own the lock after notifying a waker so that I can continue mutating the protected data structure and possibly notifying other condvars that use the same mutex (note that you can have multiple condvars sharing a mutex).

condvar.wait has to know which mutex you're working with, because (by the definition of a condition variable) it releases that mutex until it's notified, and reacquires the mutex before it returns.

This is why Rust has both block structure:

{
    let mut site = site.lock().expect("poison");
    site.pending_arrivals += 1;
    if site.pending_arrivals > 10 {
        waiting_arrivals.notify_one();
    }
    site.assign_rooms();
    if site.is_full() {
        transfer_out.notify_one();
    }
}

where the lock is released at the end of the block, and drop to let you release the guard in normal control flow:

let mut arrivals = arrivals.lock().expect("poison");
arrivals.pending_arrivals += 1;
if arrivals.pending_arrivals > 10 {
    waiting_arrivals.notify_one();
}
drop(arrivals);
let mut residents = residents.lock().expect("poison");
residents.assign_rooms();
if residents.is_full() {
    transfer_out.notify_one();
}
drop(residents);

You thus have two good options for your RAII guard - you can rely on blocks to release it at the end of the block, or you can use drop to say "it goes away now". You will note, BTW, that releasing the lock before I call notify_one in the block version would be completely wrong - I'm still using it at that point.

12

Inside the call to notify_*

13

Why would calling drop before notify be wrong?

14

But the call to notify does not take a lock guard, so can't release it. And in a lot of cases, you want to notify but still hold the lock.

You could try and justify adding notify_and_unlock methods to CondVar, which take the associated guard and release it - that's certainly a plausible solution - but then you'd need to justify that.

Perhaps look for uses of CondVar in all code referenced by https://lib.rs/ (or even better, all code on https://crates.io) and see how many cases would be simplified by a notify_and_unlock method?

15

It doesn't but it is easily imaginable that it could to be parallel with wait?

16

What are the possible scenarios that I would like to hold on to the lock after calling notify_*? What is the purpose of holding to lock after calling notify?
Surely if I call notify then I want to let other thread know that I'm done and some condition happen that the other thread can continue working?

17

Because I'm not necessarily done with the lock when I call notify - I may have more work to do.

In my site case (the block form), I'm doing two distinct things while holding the lock - one involved dealing with pending arrivals, and doing something if I have too many, the other involves working out where on my site everyone goes. Both of these need me to hold the lock, and I may have a logic error if handling pending arrivals runs concurrently with assign_rooms (e.g. I might turn people away when I could in fact fit them in after assigning rooms to the current residents - maybe move someone who had a double room for one person to a single room, and now I can move an arriving couple into the double room).

Further, I have two condvars that are sharing one mutex, and I need to be able to notify both of them. Which of them gets to release the lock? Or would you expect me to rewrite that code as:

{
    let mut site = site.lock().expect("poison");
    site.pending_arrivals += 1;
    site.assign_rooms();
    if site.pending_arrivals > 10 {
        waiting_arrivals.notify_one(site);
    }
    site = site.lock().expect("poison");
    if site.is_full() {
        transfer_out.notify_one(site);
    }
}

adding a second acquisition of the lock (thus more time spent in contention) for no reason other than to comply with the new interface?

That's why I suggested looking at a big corpus of code (such as lib.rs or crates.io) and seeing how many uses of CondVar would be simplified by a notify_one_and_unlock(guard) type of method. There exist cases in my codebases where such a method is not usable, but I'm willing to be convinced by data that there are more cases where such a method is appropriate than not.

18

Firstly, I may be notifying more than one thread via multiple condvars that all share one mutex.

Secondly, I may have more work to do under the lock after determining that I need to notify a condvar. It's convenient to be able to notify as soon as I see that such a notification is appropriate, and then to continue working before I release the lock, rather than having to record that I need to notify a given condvar, and then having to wait until I'm ready to unlock to do that notification.

Finally, I don't have to hold the lock to notify a condvar. This is useful when the data protected by the lock is not actually part of my condition for continuing; in the thread that waits, I take the lock on the data I'm going to process, I then wait on the condvar telling me I'm allowed to go, and then I go when the condvar is notified. I might, for example, notify the condvar just before I panic, so that if I was holding the lock, you get a poison result and can handle that.

19

It's like car without wheels. Yes, it's easy to imagine. yes, it's easy to make. You can even paint it different colors.

The only trouble: it would be really hard to sell it.

Monitors (usually called conditional variables because that's how Sun called them in the end of last century) behave in a certain way.

Rust certainly can implement something else in place of monitor and call it a conditional variable, but that would violate principle of least astnonishment.

The entire point of monitor is to play that ping-pong with mutexes and notifications.

That you can use it for something else (pure notification where two threads may work in parallel after notification) is just a bonus.

If you want some different behavior then you shouldn't call what you are making a condvar.

1 Like
21

I have given you an example where I have three threads operating on a single resource. In what way is it suboptimal to have one mutex for a single resource shared by three threads, where a change to that resource may result in wanting to wake up both of the other threads?