You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Is your feature request related to a problem? Please describe.
AsyncLazy<T> currently never cancels its value factory even if all callers of GetValue or GetValueAsync have canceled their token.
While this makes for a simple value factory that never cancels and only runs at most once, it can be inefficient when all callers lost interest and the value factory no longer needs to run.
Describe the solution you'd like
Add support for a value factory that takes CancellationToken as a parameter.
When all outstanding GetValue and GetValueAsync callers cancel their own tokens, cancel the value factory.
The value factory is subject to re-invocation only after it throws OperationCanceledException as a result of a canceled token passed to it. If a new GetValueAsync caller comes along while a prior the value factory invocation is still running, that caller must await the completion of that value factory before invoking it again. If the value factory completes successfully, use (and retain) its value. If the value factory cancels, GetValueAsync may re-invoke the factory.
The invariants would be:
A value factory that takes no CancellationToken would never be invoked more than once.
A cancelable value factory will never be invoked concurrently with itself.
A cancelable value factory will never be invoked more than once except after the prior call throws OperationCanceledException and the CancellationToken we provided them is canceled. Note in this check we do not compare the token with OperationCanceledException.CancellationToken because the implementation detail of the value factory may have combined our token with another so its identity may have changed even though it canceled ultimately in response to our token.
Consider exposing this additional functionality via a static factory method on AsyncLazy<T> instead of on a public constructor if it means we can avoid adding fields to the AsyncLazy<T> class, since this class is used a lot and each added field will add memory pressure to apps that use it, such as VS.
Describe alternatives you've considered
We considered running the value factory concurrently with itself after a prior invocation was canceled in order to expedite the result to a subsequent caller. This was rejected for a few reasons:
Writing a value factory that is concurrency-safe seems like a high bar that would lead to buggy code in those that use AsyncLazy<T>.
A subsequent caller that awaits the "canceled" value factory before invoking it again may in fact find that the initial invocation completed successfully rather than responding to the cancellation, in which case the fastest and most efficient thing was in fact to wait for the first invocation to complete.
Running the value factory concurrently may end up with multiple values produced. If the values require disposal or the factory had side-effects, this could result in leaks or other undesirable behavior.
Additional context
This design was hashed out with @matteo-prosperi and @sealslicer.
The text was updated successfully, but these errors were encountered:
This would be beneficial. TypeScript, for example, uses AsyncLazy but cannot cancel the work that it does, even if there are no interested parties who care. This can definitely lead to more work than necessary happening on the server side.
An example of this is what happens when a LightBulb comes up in VS. As the user arrows down through the items, teh preview window attempts to come up to show what that refactoring/fix would do. That ends up kicking off the work to compute that information, which cannot be canceled, even if the user moves to the next item. This is not ideal as it can force a lot of work on the server that contends with the actual work that is most important to the user.
I've started working on this. Since it doesn't appear to be urgent, but would be useful, it's one of my 'backburner' tasks. Let me know if this becomes urgent.
Is your feature request related to a problem? Please describe.
AsyncLazy<T>
currently never cancels its value factory even if all callers ofGetValue
orGetValueAsync
have canceled their token.While this makes for a simple value factory that never cancels and only runs at most once, it can be inefficient when all callers lost interest and the value factory no longer needs to run.
Describe the solution you'd like
Add support for a value factory that takes
CancellationToken
as a parameter.When all outstanding
GetValue
andGetValueAsync
callers cancel their own tokens, cancel the value factory.The value factory is subject to re-invocation only after it throws
OperationCanceledException
as a result of a canceled token passed to it. If a newGetValueAsync
caller comes along while a prior the value factory invocation is still running, that caller must await the completion of that value factory before invoking it again. If the value factory completes successfully, use (and retain) its value. If the value factory cancels,GetValueAsync
may re-invoke the factory.The invariants would be:
CancellationToken
would never be invoked more than once.OperationCanceledException
and theCancellationToken
we provided them is canceled. Note in this check we do not compare the token withOperationCanceledException.CancellationToken
because the implementation detail of the value factory may have combined our token with another so its identity may have changed even though it canceled ultimately in response to our token.Consider exposing this additional functionality via a static factory method on
AsyncLazy<T>
instead of on a public constructor if it means we can avoid adding fields to theAsyncLazy<T>
class, since this class is used a lot and each added field will add memory pressure to apps that use it, such as VS.Describe alternatives you've considered
We considered running the value factory concurrently with itself after a prior invocation was canceled in order to expedite the result to a subsequent caller. This was rejected for a few reasons:
AsyncLazy<T>
.Additional context
This design was hashed out with @matteo-prosperi and @sealslicer.
The text was updated successfully, but these errors were encountered: