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

Create async Send alternative #56

Merged
merged 5 commits into from
Mar 17, 2022
Merged

Create async Send alternative #56

merged 5 commits into from
Mar 17, 2022

Conversation

nappa85
Copy link
Contributor

@nappa85 nappa85 commented Mar 15, 2022

As asked in #55, there is need for a Send version of the async builders.
This PR introduces it aside of standard builders, this way old code won't be changed and it's up to the user to choose the right variant.
Obviously comments are welcome

@someguynamedjosh someguynamedjosh merged commit a20705a into someguynamedjosh:main Mar 17, 2022
@someguynamedjosh
Copy link
Owner

I have merged this but will avoid publishing it until there is confirmation that this is safe to do.

@someguynamedjosh
Copy link
Owner

@petrosagg as the original author of the async builder code, can you confirm or deny if allowing Send on the builder is safe?

@petrosagg
Copy link
Contributor

This looks perfectly safe to me! It's a bummer the type system forces us to have two separate methods but I don't think this can be fixed without removing the Box<dyn Future<..>>. It's the same reason the async_trait crate has an explicit option to control Send, because internally it also uses Pin<Box<dyn Future<..>>> types https://docs.rs/async-trait/latest/async_trait/#non-threadsafe-futures

One day..

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

Successfully merging this pull request may close these issues.

None yet

3 participants