weldsorm / welds

BSD 3-Clause "New" or "Revised" License
119 stars 6 forks source link

Unable to spawn tasks using welds with Tokio RT #15

Closed NexRX closed 9 months ago

NexRX commented 10 months ago

Hi all, sorry to post another issue so soon. I actually tried to clone and fix this issue my self but I cant figure out the crux of the issue here. Mostly because I don't really understand runtimes under the hood I suspect.

Anyway, I'm trying to run a poem web server and I'm sharing the database pool via a reference passed into my endpoints function handler like so:

#[OpenApi()]
impl UserApi {
    #[oai(path = "/", method = "post")]
    async fn create_user(
        &self,
        pool: Data<&ClientPool>,
        user: Json<User>, // Json implments deref btw and User derives all the welds stuff
    ) -> ApiResponse{
        let mut user = user.uncreated(); // Function i wrote just convert self to a uncreated DbState<Self>.

        user.save(*pool).await.unwrap() // <-- Problem function `save`

        let id = user.into_inner().await.id;
        Success::rtn(Message::success(id).json())
    }
}

It produces a really long error but Ill give the short version here and the long version in a pastebin here:

error[E0277]: `dyn std::future::Future<Output = std::result::Result<std::vec::Vec<sqlx_postgres::row::PgRow>, anyhow::Error>>` cannot be sent between threads safely

Removing the function call does remove the error and i tried to make all the generics used in welds explicitly implement send if they didn't already but that didn't work.

Removing functions inside save(pool) like

insert::insert_one(&mut self.sql_buff, &mut self.inner, conn).await?;

Will also fix the error (and breaks the function ofc) so it's obviously the fact that yes, those functions do capture reference or types. But i'm not really sure what would have to change in weld to fix that since making. Making many things as I can impl send didn't fix it. If you guys know how to fix it and pointers is enough on what a solutions is, I wouldn't mind contributing.

It's Christmas so I'll leave this issue here as I don't expect anything back untill everyones done being merry πŸŽ…πŸ» So have a Merry Christmas! πŸ˜†

NexRX commented 10 months ago

I spent a little more time figuring it out and I guess its seems like a incompatiability with tokio. You don't need a poem web server to reproduce this. Instead, just spawn a task with tokio and go from there.

pub async fn create(mut user: DbState<User>) { // Or any DbState<T>
   let pool = client("url").await.unwrap();

   tokio::spawn(async move { user.save(&pool).await });
}

I see in the examples that async-std is used. Is async-std a requirement or is it possible to use tokio rt? :)

lex148 commented 9 months ago

Good morning @NexRX,

You had a fun issue here :) The root of the problem was while welds is fully async, There were a couple places that were not marked as Send so Tokio couldn't share the task between threads. Interestingly Tokio could figure it out when sticking to the async call in main. Thanks for the help finding this one.

The fix is on the main branch now. I plan on releasing to crates.io soon.

NexRX commented 9 months ago

Good morning @NexRX,

You had a fun issue here :) The root of the problem was while welds is fully async, There were a couple places that were not marked as Send so Tokio couldn't share the task between threads. Interestingly Tokio could figure it out when sticking to the async call in main. Thanks for the help finding this one.

The fix is on the main branch now. I plan on releasing to crates.io soon.

Awesome stuff. Thanks for breaking it down. It seemed like that but I couldn't see where Send wasn't required. I can use git main until crates release is done so no rush. Thanks again!