Borrowing and Refactoring

1

I find myself in this situation often: I refactor a bit of common code into a helper function, then run into borrowing errors.

Very often, one cannot - as in most other languages - simply move a bit of (referentially-transparent) code from the body of one function to a standalone function.

Case in point:

    fn lock(&self, mutex: Mutex<V>) -> MutexGuard<V> {
        match mutex.try_lock_for(Duration::from_secs(60)) {
            Some(guard) => guard,
            None => panic!("Mutex lock timed out"),
        }
    }

(Note: These are the parking_log synch artifacts, not std lib.)

One cannot do this, of course - we get

75 |             Some(guard) => guard,
   |                            ^^^^^ returns a value referencing data owned by the current function

I don't really need to wrap this return (Box or Arc), do I?

I'm sure I'm missing some basic tricks. I hope so. What's a general type of solution for doing this?

2

You are taking Mutex by value, it gets dropped at the end of the function. MutexGuard cannot reference a dropped mutex.

Solution: mutex &Mutex<V> instead of Mutex<V>.

3 Likes
3

I was about to say I DO need to Box here (in general for something not Clone), but that does work also!

4

By the way, you are not using &self, so it would better be a free standing function or static method. Borrowing &self may potentially cause borrow-checker errors too.

5

Box wouldn't help here

6

(I'm wondering if some other IDEs handle this better. I'm using Zed and its extract into function command does not know stuff like this.)

7

As @Ddystopia said, Box won't help. The reason you're getting an error is because you're transferring ownership to the function by not borrowing the Mutex. You'll need to understand ownership to use Rust. Please ask any questions you have about ownership in this situation (it won't work to rely on the IDE to always give you what you want).

8

The solution like this:

//                    v
fn lock(&self, mutex: &Mutex<V>) -> MutexGuard<V> {
    match mutex.try_lock_for(Duration::from_secs(60)) {
        Some(guard) => guard,
        None => panic!("Mutex lock timed out"),
    }
}

will actually fail (Playground), because of lifetime elision rules. The lifetimes are elided to this:

fn lock<'this, 'mutex>(
    &'this self, mutex: &'mutex Mutex<V>
//              vvvvv
) -> MutexGuard<'this, V> { ... }

The elision rules assume the returned guard should borrow from self, when it actually borrows from the Mutex.

In your OP, you can remove the &self parameter entirely, since self is not used within the function. Rust isn't Java, you can just have free functions without defining an associated class/struct. If you do actually need self for something, the right lifetime signature is this:

fn lock<'this, 'mutex>(
    &'this self, mutex: &'mutex Mutex<V>
//              vvvvvv = ^^^^^^
) -> MutexGuard<'mutex, V> { ... }

"Hidden" lifetimes in types like MutexGuard<'a, T> are often footguns, so much so that there's a lint for it: #![deny(elided_lifetimes_in_paths)].

2 Likes
9

I did end up removing &self, actually.

(However, still needed lifetime specs for cases like this:

    fn lock<'a>(key: &'a K, mutex: &'a Mutex<V>) -> MutexGuard<'a, V> {
        match mutex.try_lock_for(Duration::from_secs(60)) {
            Some(guard) => guard,
            None => panic!("Mutex lock acquire timed out; key: {:?}", key),
        }
    }

)

10

I understand ownership better than it sounded like here. :cry:

1 Like
11

That code correlates the lifetime of key with the lifetime of the MutexGuard, which is unnecessary. Removing the lifetime on key and letting it be elided should work:

fn lock<'a>(key: &K, mutex: &'a Mutex<V>) -> MutexGuard<'a, V> {
    ...
}
The equivalent signature without any elision 
fn lock<'key, 'mutex>(key: &'key K, mutex: &'mutex Mutex<V>) -> MutexGuard<'mutex, V>

Sidenote: the body can be written as

mutex.try_lock_for(Duration::from_secs(60))
    .unwrap_or_else(|| panic!("Mutex lock acquire timed out; key: {key:?}"))
1 Like