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

Use Arc for SeaRc with feature flag #105

Closed
tqwewe opened this issue Aug 15, 2021 · 5 comments
Closed

Use Arc for SeaRc with feature flag #105

tqwewe opened this issue Aug 15, 2021 · 5 comments

Comments

@tqwewe
Copy link

tqwewe commented Aug 15, 2021

Given the following function in a tokio runtime:

async fn update(
    db: &DatabaseConnection,
    active_model: model::ActiveModel,
) -> Result<bool, Box<dyn std::error::Error>> {
    Entity::update(active_model)
        .exec(db)
        .await
        .map_err(Box::new)?;

    Ok(true)
}

An error is thrown due to SeaRc not implementing Send.

error: future cannot be sent between threads safely
   |
76 |       ) -> Result<bool, Box<dyn std::error::Error>> {
   |  ___________________________________________________^
77 | |         use models::coupons::Entity as Coupon;
78 | |
79 | |         let res = Coupon::update(coupon_update)
...  |
84 | |         Ok(true)
85 | |     }
   | |_____^ future created by async block is not `Send`
   |
   = help: within `TableRef`, the trait `std::marker::Send` is not implemented for `SeaRc<(dyn Iden + 'static)>`

pub use std::rc::Rc as SeaRc; should be updated to use Arc instead of Rc, either perminently or through a feature flag.

@tyt2y3
Copy link
Member

tyt2y3 commented Aug 15, 2021

I think you encountered a problem in SeaORM.
Yes I was aware of the async quirk. I still believe we can keep the query building stage on the same thread, then we would not requite Arc.
But I do agree we can add a feature flag for this as a workaround, and enable it by default in SeaORM.

@tqwewe
Copy link
Author

tqwewe commented Aug 15, 2021

Yes this is an error I encountered in SeaORM. I don't know of any workaround, it seems like the only option is that SeaRc to be changed to Arc. I agree that enabling this in SeaORM by default would be best 👍

tyt2y3 added a commit that referenced this issue Aug 15, 2021
tyt2y3 added a commit that referenced this issue Aug 15, 2021
@tyt2y3
Copy link
Member

tyt2y3 commented Aug 15, 2021

0.14.0

@tyt2y3 tyt2y3 closed this as completed Aug 15, 2021
@tqwewe
Copy link
Author

tqwewe commented Aug 15, 2021

Thanks for the commits for this.

Unfortunately, I get an error still with SeaORM with my same code:

the trait `std::marker::Send` is not implemented for `(dyn Iden + 'static)`

I have enabled the "thread-safe" flag and can confirm it's using Arc. There may need to be some other code updates to resolve this.

@tyt2y3
Copy link
Member

tyt2y3 commented Aug 15, 2021

Please open an issue in SeaORM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants