Question to borrow checker on RwLock

help
1

Hello,

I don't understand why there is a problem for the borrow checker here:

error[E0515]: cannot return value referencing function parameter `data`
  --> src/main.rs:40:17
   |
40 |     let entry = data.read()?;
   |                 -----------^
   |                 |
   |                 returns a value referencing data owned by the current function
   |                 `data` is borrowed here

Is this something not yet implemented to fiddle out? But I don't see any memory problem here.
If I don't oversee anything, there is nothing moved or referenced from the RwLockReadGuard<Entry> afterwards. The guarded String gets "cloned" And File is something which should be moveable outside of the guard.

When I use an if let it works...

fn work(data: Arc<RwLock<Entry>>) -> Result<File, Box<dyn std::error::Error>> {
    let file = if let Ok(entry) = data.read() {
        let file = open_tmp_file(&entry)?;
        Some(file)
    } else {
        None
    };

    Ok(file.expect("file"))
}

But the data.write()? above has no problem...? What's going on here?
Thanks

2

The problem comes from the lifetime of the dyn Error you return with ?. data.read()? returns a LockResult<RwLockReadGuard<'a, Entry>>, where 'a is the lifetime of the implicit data borrow. This is equivalent to a Result<RwLockReadGuard<'a, Entry>, PoisonError<RwLockReadGuard<'a, Entry>>>. The ? operation first extracts the error, then converts it to the error in the return type using .into(). Look at the From impl which allows this:

impl<'a, E: Error + 'a> From<E> for Box<dyn Error + 'a> { ... }

Here, E is RwLockReadGuard<'a, Entry>, which only has lifetime 'a. Therefore, .into() can only return a Box<dyn Error + 'a>, not a Box<dyn Error + 'static>, since the PoisonError inside the Box is still referencing the RwLock. Since work() requires a Box<dyn Error + 'static> to be returned, the compiler produces an error.

The solution is to either .unwrap()/.expect() the PoisonError, or to .map_err() it into some 'static error type that does not borrow anything. Personally, I'd use .unwrap() or .expect(), since a PoisonError signifies that some other part of your code has already panicked while operating on the Entry, and it may be in an unexpected state.

3

Please don't use static mut, it's incredibly unsafe! Not surprisingly, your usage of it is wrong: it has a race condition because if let Some(data) = unsafe { &DATA } isn't synchronized.

For a global variable, Arc is unnecessary, too. (It's already shared because it's a global static – you always have a single instance which you can access from anywhere.)

Your example can be rewritten soundly, without any unsafe or reference counting, or expect()ing in a Result-returning function:

static DATA: RwLock<Option<Entry>> = RwLock::new(None);

fn main() -> Result<(), Box<dyn std::error::Error>> {
    *DATA.write().unwrap() = Some(Entry::default());

    let mut entry = DATA.write()?;
    if let Some(entry) = entry.as_mut() {
        entry.s = "foorbar".into();
        create_tmp_file(&entry)?;
    }
    drop(entry); // release lock

    work(&DATA)?;

    Ok(())
}

fn work(data: &RwLock<Option<Entry>>) -> Result<File, Box<dyn std::error::Error + '_>> {
    let entry = data.read()?;
    let entry = entry.as_ref().ok_or("missing file")?;
    let file = open_tmp_file(&entry)?;

    Ok(file)
}

Better yet, you could use once_cell::sync::Lazy instead of an explicit Option and explicit initialization, for more readable and less verbose code:

static DATA: Lazy<RwLock<Entry>> = Lazy::new(RwLock::default);

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let mut entry = DATA.write()?;
    entry.s = "foorbar".into();
    create_tmp_file(&entry)?;
    drop(entry); // release lock

    work(&DATA)?;

    Ok(())
}

fn work(data: &RwLock<Entry>) -> Result<File, Box<dyn std::error::Error + '_>> {
    let entry = data.read()?;
    let file = open_tmp_file(&entry)?;

    Ok(file)
}
2 Likes
4

Yes thanks, but it hasn't to do anything with the borrow problem.
I've tried to reproduce this with this stripped down code.
I use this nowhere else.
There is no race condition in this sample code, because there is no thread usage or anything async in it.

5

Oh yes, it points to the question mark :wink:
Didn't see that before, thought only about data and entry.

Thanks.

But there is nevertheless a strange problem...
There is no difference in write vs. read lock in this area.
And in the meantime I've found this strange thing...

This works...

[... deleted ...]

So to be clear... why it works here in main() with the same code and not in work()?

You've written...

Since work() requires a Box<dyn Error + 'static> to be returned, the compiler produces an error.

Doesn't require main() the same?

6

It works in main() because the &DATA borrow is 'static. It doesn't work in work() because data is a local variable, which cannot be borrowed for 'static. (Note that if you actually did this, then the RwLock would remain locked until the Box<dyn Error> is dropped.)

2 Likes
7

You're right...

fn work(data: &'static Arc<RwLock<Entry>>) -> Result<(), Box<dyn std::error::Error>>

would compile.

8

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.