rust: accept fallible futures in `async_support::spawn`

[dicej]

Currently, async_support::spawn accepts an impl Future<Output = ()>, but it should probably take something like impl Future<Output = anyhow::Result<()>> instead, since the most common cases for spawned Futures involve fallible I/O, and forcing the application to .unwrap() (and thus trap) on any error is not a great default experience.

In the case where such a Future returns an error, we can convert it to a debug string, pass that string to ErrorContext::new, and then close any open streams and/or futures (read or write ends) with that error-context.

[dicej]

Alternatively, it could be impl Future<Output = Result<(), ErrorContext>>, but then we'd want to make sure wit-bindgen-rt has some form of interop between ErrorContext and anyhow::Error and make sure the ? operator "just works" for most cases.

[pchickey]

if ErrorContext is a pub type that impls std::error::Error, then you can use anyhow::Error in the interface to the user's Rust, and try downcasting it to ErrorContext in the implementation of the bindings.

[alexcrichton]

I'm less certain on what would be possible to do with an error in this case. You say we can close open futures/streams but how would that be enumerated? How would the set of futures/streams tied to a Rust-spawned task be enumerated to be closed with the error?

[dicej]

I'm less certain on what would be possible to do with an error in this case. You say we can close open futures/streams but how would that be enumerated? How would the set of futures/streams tied to a Rust-spawned task be enumerated to be closed with the error?

We have a HANDLES HashMap with all the stream and future handles for all tasks, but yeah, there's no way to tie a stream or future to a given task except when it's being actively read or written -- otherwise, they're global resources available to any task AFAICT.

@lukewagner I seem to recall last time we discussed this that we thought it would be possible to enumerate futures and streams on a per-task basis, but now I'm not sure if it is. Do you remember?

[dicej]

Wild idea: we could store the ErrorContext in a thread-local variable prior to dropping the spawned Future after it has returned Poll::Ready(Err(_)), and any handles which are closed while that thread-local is set will be closed with the ErrorContext. Then, when we're finished dropping the Future, we clear that thread-local. Thoughts?

[dicej]

I.e. we wouldn't be dropping all the streams and futures which "belong to the task" -- just the ones that literally (in the Rust ownership sense) belong to that specific spawned Future, which is what we really want anyway, I believe.

[alexcrichton]

Personally I want to more-or-less get rid of HANDLES or otherwise only rely on it in very few circumstances. We could probably track information to map back to a component-model task but we still would be unable to track them at a per-Rust-task basis which I think is what this issue needs. Even if we could enumerate futures/streams for a component-model task that's not sufficient for Rust-level tasks.

Thoughts?

Personally I feel like that's bending over too much to get this API to work. The alternative is not supporting spawning with a Result return value which is how other async runtimes work in Rust as well

[dicej]

Personally I want to more-or-less get rid of HANDLES or otherwise only rely on it in very few circumstances.

Fair enough; my thread-local ErrorContext idea wouldn't require it anyway.

Personally I feel like that's bending over too much to get this API to work. The alternative is not supporting spawning with a Result return value which is how other async runtimes work in Rust as well

I might sketch out a spawn_fallible function anyway just to see how it feels from a developer perspective. I agree that it's a bit magical, but I think it would be straightforward to implement and be an ergonomic win for many common cases.

[lukewagner]

@dicej After the waitable-sets change, stream/future ends don't have any semantic connection to tasks, so I can't think of any CABI built-ins we could add for enumerating them. I expect one could maintain information in a guest memory array managed by the async runtime; the main question is whether you want to allow a writable end to outlive the async export call that created it and, if so, how that gets expressed in the source code.

[dicej]

Thanks, @lukewagner -- makes sense. The more I've thought about it, the more I think it's not really a matter of which streams/futures belong to a given component-level task anyway -- it's really only a matter of which streams/futures a spawned Rust Future has taken ownership of, and thus which ones need to be closed with an error if that Future produces a Poll::Ready(Err(_)) -- i.e. they'll be closed either way via a recursive drop, so we might as well close them with an error since we know that's what happened.