StephenCleary / AsyncEx

A helper library for async/await.
MIT License
3.53k stars 356 forks source link

Add cancellation support to `AsyncLazy` #287

Open julealgon opened 1 month ago

julealgon commented 1 month ago

We currently have many uses of AsyncLazy but they are not participating in cooperative cancellation because passing a CancellationToken at "await time" is not possible.

Would it be possible to provide an extension to pass in a CancellationToken before awaiting the AsyncLazy? Perhaps a Task<T> WithCancellationAsync(CancellationToken) method, so the caller can provide a token, like this:

var value = await myAsyncLazy.WithCancellationAsync(cancellationToken);

And then have an overload on the constructor to allow providing a Func<T, CancellationToken, Task> that can propagate this token to the underlying factory.

I'm aware I could "bake in" a cancellation token via a closure when specifying the factory, but that is not only convoluted, it is also common that the place that defines the factory doesn't have access to the CancellationToken object yet, so it can't always be used. It also doesn't work for anything that is cross-request, for example an AsyncLazy inside of a singleton registration in a WebAPI: in that case, I want the first request to the object to define provide CancellationToken, so it cannot be provided via a closure at all.

I also could couple the AsyncLazy await with a separate timeout task, but then even when the timeout is reached, the actual factory will keep executing behind the scenes and will not be cancelled. This gets me some benefits (the caller can be cancelled) but it is not perfect either.

StephenCleary commented 1 month ago

That's interesting. I'm thinking about edge cases here:

What's your actual use case? To me, the WaitAsync approach would be fine for shared singletons, because your code will most likely need them soon anyway.

julealgon commented 1 month ago

That's interesting. I'm thinking about edge cases here:

  • If A starts awaiting with a CT and then B starts awaiting without a CT, then B can observe a cancelled await even though it didn't specify a CT.
  • If A starts awaiting without a CT and then B starts awaiting with a CT, would the CT be ignored? Alternatively, we could always pass a CT to the function and then dynamically link its CTS to the future CT. In which case we'd want to do that for all future CTs. But then if any of the CTs are cancelled, they'd all be cancelled. Seems like we might want a "cancel if all" instead of a "cancel if any" semantic.

Yeah, I thought about this and the more I look at it, the more I'd actually prefer 2 distinct APIs:

Call the second one something like... CancellableAsyncLazy, for example. It requires that the initialization factory takes a ct (even if it is ignored... then it's up to the consumer who decided to ignore it and he doesn't get cancellation benefits), and it always takes a ct to await (so it wouldn't actually be awaitable directly like AsyncLazy is today, but only expose either a WaitAsync(CancellationToken) or a GetValueAsync(CancellationToken) method.

So,

API 1:

AsyncLazy<int> lazyValue = new(() => GetMyServiceValueAsync());
var value = await lazyValue;

API 2:

CancellableAsyncLazy<int> lazyValue = new(ct => GetMyServiceValueAsync(ct));
var value = await lazyValue.GetValueAsync(cancellationToken);

I think having separate APIs like this removes some of the corner cases you listed and thus makes for a more robust and explicit experience, and would reduce the complexity around handling linked tokens too.

What's your actual use case? To me, the WaitAsync approach would be fine for shared singletons, because your code will most likely need them soon anyway.

Basically, all of our use cases are for cache-like behavior in the same class, so we always have either the token always available on the caller, or never available. And we are actively switching most of our public APIs to be cancellable to allow for proper timeouts and cancellations at the API levels etc, so the expectation is that every single case will eventually be converted to use a cancellation token.

StephenCleary commented 1 month ago

And are you thinking "cancel if all" or "cancel if any"?

julealgon commented 1 month ago

And are you thinking "cancel if all" or "cancel if any"?

I think "cancel if all" does make the most sense here like you mentioned earlier otherwise one call would end up affecting others which would feel weird.

julealgon commented 2 weeks ago

@StephenCleary you may already be aware of this, but I'm sharing here just in case.

I was looking at the new HybridCache docs on the what's new in AspNET 9 page, and found this interesting part:

The cancel token here represents the combined cancellation of all concurrent callers—not just the cancellation of the caller we can see (that is, token).

It might be beneficial to implement the cancellation on AsyncLazy in a similar manner for consistency.