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

Support async builders? #57

Closed
fzyzcjy opened this issue Jun 15, 2024 · 17 comments
Closed

Support async builders? #57

fzyzcjy opened this issue Jun 15, 2024 · 17 comments

Comments

@fzyzcjy
Copy link

fzyzcjy commented Jun 15, 2024

Hi thanks for the library! I do hope the builder function can support async functions.

Context: I am considering using this package to implement returning borrowed types in https://github.com/fzyzcjy/flutter_rust_bridge. Thus the builder can be arbitrary async code.

@Voultapher
Copy link
Owner

Hi, thanks for reaching out. Do I understand correctly, you have a dependent construction logic that involves async calls, right?

If possible, I'd like to avoid the async coloring issue of having multiple variants of all construction functions. There are also some question about variance across await points, where I'm not sure what a sound API would look like.

Would a spawn_blocking work for your use-case?

@fzyzcjy
Copy link
Author

fzyzcjy commented Jun 15, 2024

Hi, thanks for the reply! Ok I see...

Would a spawn_blocking work for your use-case?

Well no, because https://github.com/fzyzcjy/flutter_rust_bridge needs to support arbitrary user function. If we require users to do spawn_blocking, then indeed we lose the benefits of async (e.g. not taking a whole thread).

@Voultapher
Copy link
Owner

Could you please outline specifically what kind of API you would need.

@fzyzcjy
Copy link
Author

fzyzcjy commented Jun 15, 2024

Sure. For example, the README example says:

impl NewStructName {
    fn new(
        owner: Owner,
        dependent_builder: impl for<'a> FnOnce(&'a Owner) -> Dependent<'a>
    ) -> NewStructName { ... }

but now it would be great to be

impl NewStructName {
    fn new(
        owner: Owner,
        dependent_builder: impl for<'a> FnOnce(&'a Owner) -> Pin<Box<dyn Future<Output = Dependent<'a>>>> // <- support passing in async function. not necessarily have API like this.
    ) -> NewStructName { ... }

Indeed just something like https://docs.rs/ouroboros_examples/latest/ouroboros_examples/ouroboros_impl_chain/struct.Chain.html#method.new_async

@Voultapher
Copy link
Owner

It would need to async fn new( right?

@Voultapher
Copy link
Owner

Also I wonder if a minimal solution that only duplicates the new constructor is enough for a first version until people ask for more.

@Voultapher
Copy link
Owner

@steffahn what's you take, do we run into any variance/soundness issues with such an API?

@fzyzcjy
Copy link
Author

fzyzcjy commented Jun 15, 2024

It would need to async fn new( right?

Oh yes!

Also I wonder if a minimal solution that only duplicates the new constructor is enough for a first version until people ask for more.

Sorry not quite get it...

Btw, no worries about this issue, because if #58 cannot be solved, then I unfortunately cannot use this package :( Though I think it is great and abstracts out unsafe things with well tested code!

@steffahn
Copy link
Contributor

IIRC ouroboros has async construction APIs for a while already, so if we'd just mirror those, it might be sound (well.. or both are unsound). I haven't looked at the details myself yet.

@Voultapher
Copy link
Owner

If there are other users that request this, I will take another look.

@tliron
Copy link

tliron commented Jan 17, 2025

@Voultapher, astounding library, thank you for it!

I hate to comment on a closed issue, but it seems you closed this one as "completed". Is there really async support now, or did you mean to close it as "won't implement"?

As it stands, I am using both self_cell and Ouroboros in my project, the former for blocking pathways and the latter just for async. I would honestly prefer self_cell for both. I've tried pretty hard to get self_cell to work with Pin::box(async move) closures, as in Ouroboros, but reached the limits of my understanding. :) If there is a way to do it without changing self_cell, just including an example would be enough to "close as completed", I reckon.

Is this a feature you'd rather not have in self_cell at all, or would you be willing to accept help to develop it?

@Voultapher
Copy link
Owner

@tliron realistically the draining part is maintaining the feature and documentation and looking out for soundness bugs, not really the developing part. I might take another stab at this at some point in the future , but for now it's not planned.

@tliron
Copy link

tliron commented Jan 17, 2025

I understand and sympathize.

In your opinion, is it possible to achieve with self_cell's design as it stands, without modifications? In that case, it would be enough to include an example. I think that would involve no further maintenance beyond the inclusion of it.

If you think it's possible I might pursue trying to create such an example, but I'd rather not even try if you think it's plain impossible. What is your opinion?

@Voultapher
Copy link
Owner

I don't think there is an elegant way to do it without modifying the source code, something like futures::block_on could be used but won't compose nicely.

Could you please provide your specific use-case so that I can check that if I would add this feature that it would even solve our use-case.

@tliron
Copy link

tliron commented Jan 18, 2025

We might be thinking of a different use case, because futures::block_on cannot work in an async situation. I think you are thinking about bridging async code to blocking.

My exact situation is development of the read-url crate. Specifically for opening zip: URLs, I need to "carry" the zip crate's data with the reader. For the blocking variant, self_cell is terrific! Here is that code. As you can see, there are "pairs" of dependents due to the architecture of the crate I am using, but self_cell expresses these pairs very nicely. For the async variant, I had no choice but to use Ouroboros. Here is the async code. As you can see, Ouroboros lets me use async closures (with Pin<Box>) to build the dependents with a constructor function that is also async, allowing the entire flow to be async.

Here are some of my sad attempts to try to do the same in self_cell. :) I stopped working on it because I thought to check here, first.

@Daniel-Aaron-Bloom
Copy link

Just putting a +1 on this feature request. My use case is trying to store an Arc<PubsubClient> and an associated Stream from one of the functions.

@tliron
Copy link

tliron commented Apr 1, 2025

I wonder if the new async closure support added to Rust 1.85 can make this easier for us to implement.

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

5 participants