Question to borrow checker on RwLock

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

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.

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

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.

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?

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

You're right...

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

would compile.