Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lifetime issue when borrowing inside a transaction #27

Closed
3 tasks done
Tuetuopay opened this issue Oct 3, 2022 · 4 comments
Closed
3 tasks done

Lifetime issue when borrowing inside a transaction #27

Tuetuopay opened this issue Oct 3, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@Tuetuopay
Copy link

Setup

Versions

  • Rust: 1.64
  • Diesel: 2.0
  • Diesel_async: 0.1
  • Database: tested using postgres but should be database-agnostic since it's a compile-time issue
  • Operating System: Linux

Feature Flags

  • diesel: postgres
  • diesel_async: postgres

Problem Description

When capturing data in the closure + future passed to AsyncConnection::transaction(), lifetimes makes the code not compile.

The actual error given out of the box is the following:

error[E0597]: `user_id` does not live long enough
  --> src/main.rs:30:66
   |
30 |         .transaction(|conn| Box::pin(async { users::table.find(*&user_id).first(conn).await }))
   |                      ------ -------------------------------------^^^^^^^----------------------
   |                      |      |                                    |
   |                      |      |                                    borrowed value does not live long enough
   |                      |      returning this value requires that `user_id` is borrowed for `'static`
   |                      value captured here
...
40 | }
   | - `user_id` dropped here while still borrowed

The error is very confusing because, obviously, the closure and the future are consumed before the end of the main function. And the 'static lifetime makes no real sense.

I originally though the issue was due to automatic lifetimes assigned by rustc to the Future in the .transaction function: give the same lifetime to the conn and to the future, so I tried to explicit them:

// extract of `src/transaction_manager.rs`
    async fn transaction<'conn, 'fut, F, R, E>(conn: &'conn mut Conn, callback: F) -> Result<R, E>
    where
        F: FnOnce(&mut Conn) -> BoxFuture<'fut, Result<R, E>> + Send,
        E: From<Error> + Send,
        R: Send,
    {
        Self::begin_transaction(conn).await?;

With this, the compile error changes, but is as confusing as ever:

error: lifetime may not live long enough
  --> src/main.rs:30:29
   |
30 |         .transaction(|conn| Box::pin(async { users::table.find(*&user_id).first(conn).await }))
   |                       ----- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
   |                       |   |
   |                       |   return type of closure is Pin<Box<(dyn futures::Future<Output = Result<User, diesel::result::Error>> + std::marker::Send + '2)>>
   |                       has type `&'1 mut AsyncPgConnection`

At least this time the lifetimes involved make sense: '1 (i.e. the reborrowed conn) must outlive '2 (i.e. the future that uses said conn). But it makes no sense in this case that the future outlives the connection (because this is what the compiler effectively tells us, that it may outlive it).

The full code is below.

Many thanks, this is the last hurdle for me to adopt diesel 2 + diesel-async!

What are you trying to accomplish?

Running a transaction that borrows data from around it.

What is the expected output?

A binary.

What is the actual output?

A lifetime error I cannot wrap my head around.

Are you seeing any additional errors?

Steps to reproduce

main.rs:

use anyhow::Result;
use diesel::{prelude::*, table, Identifiable, Queryable};
use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};

table! {
    users(id) {
        id -> Integer,
        name -> Text,
    }
}

#[derive(Queryable, Identifiable, Debug, Clone)]
#[diesel(table_name = users)]
struct User {
    id: i32,
    name: String,
}

#[tokio::main]
async fn main() -> Result<()> {
    let mut conn = AsyncPgConnection::establish(&std::env::var("DATABASE_URL")?).await?;

    let user_id = 4242;
    // The *& is intended to make the closure capture a reference since i32 is Copy
    let user: User = conn
        .transaction(|conn| Box::pin(async { users::table.find(*&user_id).first(conn).await }))
        .await?;
    println!("Found user {user:?}");

    Ok(())
}

Cargo.toml:

[package]
name = "diesel-2-issue"
version = "0.1.0"
edition = "2021"

[dependencies]
anyhow = "1"
diesel = { version = "2", features = ["postgres"] }
diesel-async = { path = "../diesel_async", version = "0.1", features = ["postgres"] }
tokio = { version = "1", features = ["rt-multi-thread", "macros"] }

Checklist

  • I have already looked over the issue tracker for similar possible closed issues.
  • This issue can be reproduced on Rust's stable channel. (Your issue will be
    closed if this is not the case)
  • This issue can be reproduced without requiring a third party crate
@Tuetuopay Tuetuopay added the bug Something isn't working label Oct 3, 2022
@weiznich
Copy link
Owner

weiznich commented Oct 4, 2022

This is essentially a duplicate of #23

As explained there this is a limitation of the current implementation of async in rustc, which means we cannot do much to improve the situation there as long as there are not substantial improvements on the language/compiler side. As for the error message: Yes that one is not great, but again: It's something that comes from rustc, so please report this in their repository. They care for such bad error messages and that's a quite general case here.

As a workaround: Your callback needs to own all relevant values. Use something like

let user: User = conn
        .transaction(move |conn| Box::pin(async move { users::table.find(user_id).first(conn).await }))
        .await?;

Closed as this is not actionable from our side.

@weiznich weiznich closed this as not planned Won't fix, can't repro, duplicate, stale Oct 4, 2022
@Tuetuopay
Copy link
Author

Thanks. I did not think to check the GH discussions, only the issues, sorry.

Sadly the workaround is ... not that possible as I'm handed borrowed data directly. I'd need to clone it, which is not ideal at all. Oh well, that's what I'm getting for being on the bleeding edge 😄

@weiznich
Copy link
Owner

weiznich commented Oct 4, 2022

I'd need to clone it, which is not ideal at all.

I fear cloning stuff is currently the only possible solution. I'm happy to be proven wrong here, but I just do not see any other option to write such an API at all.

@Tuetuopay
Copy link
Author

Yep, in this case I cannot avoid cloning as I'm passed references. In the owned case, the usual lifetime joker that is Arc is a solution though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants