launchbadge / sqlx

🧰 The Rust SQL Toolkit. An async, pure Rust SQL crate featuring compile-time checked queries without a DSL. Supports PostgreSQL, MySQL, and SQLite.
Apache License 2.0
12.95k stars 1.23k forks source link

Transaction meets E0521: borrowed data escapes outside of method #3404

Closed tisonkun closed 1 month ago

tisonkun commented 1 month ago

Bug Description

Create a transaction, return it outside and later move back can cause lifetime checker suck.

Minimal Reproduction

pub async fn fetch_group_meta(
        &self,
        group_id: String,
        insert_on_missing: bool,
) -> MetaResult<(String, Transaction<'_, Postgres>)> { ... }

pub async fn update_group_meta(
    &self,
    txn: Transaction<'_, Postgres>,
    group_id: String,
    group_meta: String,
) -> MetaResult<()> {
    common_runtime::meta_runtime()
        .spawn(async move {
            let mut txn = txn;
            ...
            txn.commit().await?;
            Ok(())
        })
        .await?
}

reports:

error[E0521]: borrowed data escapes outside of method
   --> crates/meta/src/service.rs:215:9
    |
210 |           txn: Transaction<'_, Postgres>,
    |           ---              -- let's call the lifetime of this reference `'1`
    |           |
    |           `txn` is a reference that is only valid in the method body
...
215 | /         common_runtime::meta_runtime()
216 | |             .spawn(async move {
217 | |                 let mut txn = txn;
...   |
225 | |                 Ok(())
226 | |             })
    | |              ^
    | |              |
    | |______________`txn` escapes the method body here
    |                argument requires that `'1` must outlive `'static`

For more information about this error, try `rustc --explain E0521`.

Info

tisonkun commented 1 month ago

Since I use pool.begin to get this Transaction, I change the type sig to Transaction<'static, Postgres> and get it fixed.

However, it looks like a rust compiler bug that I must move the txn into the closure but it reports that the txn escapes ..

CommanderStorm commented 1 month ago

What is common_runtime::meta_runtime().spawn(...)? I am assuming you are using tokio::task::spawn` => that future requires a 'static lifetime. See the docs for an explaination why and how to deal with this.

Lets back off: Why do you want to use a for part of the transaction? That seems somewhat strange to me..

tisonkun commented 1 month ago

that future requires a 'static lifetime

This seems the answer. Thank you!

Why do you want to use a for part of the transaction?

I have a meta service to interact with PG in a transaction context. Previously, I pass a closure as an effective "stored process" to process data read from txn. But later I encounter some subtle closure issues (mainly about error handling, that I'd return an error to the parent of parent context or things like that) (closure is complex in Rust, isn't it?).

So I try to read the data within txn and return both the data and the txn, and then pass back the txn with second part's input to continue the transaction, as shown in the fn sig above.

CommanderStorm commented 1 month ago

What does the "meta service" do preciciely?

The context you gave does not answer why you need to spawn a new task on your runtime inside of the Transaction<'_, Postgres>. => not having that code is likely better and solves your problem as far as you have explained.

tisonkun commented 1 month ago

@CommanderStorm I agree that context is lacking.

My question is answered anyway and thanks for your time. Perhaps I can share my user story once the code above is open sourced (hopefully in a few weeks).