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

Add DisposableAsyncLazy<T> class to simplify guaranteed disposal of values #953

Open
AArnott opened this issue Nov 17, 2021 · 0 comments
Open
Assignees

Comments

@AArnott
Copy link
Member

AArnott commented Nov 17, 2021

Is your feature request related to a problem? Please describe.

When the T in AsyncLazy<T> implements IDisposable, ensuring that the T value is disposed of requires very carefully written code. Doing this properly requires:

  1. Blocking all future users of the AsyncLazy<T>
  2. Carefully disposing of the value if it ever was or might be created using such code as this:
if (lazy.IsValueCreated) {
  if (lazy.IsValueFactoryCompleted) {
    lazy.GetValue().Dispose();
  } else {
    lazy.GetValueAsync().ContinueWith(
      t => t.Result.Dispose(),
      CancellationToken.None,
      TaskContinuationOptions.OnlyOnRanToCompletion,
      TaskScheduler.Default);
}
  1. Potentially blocking on completion of the disposal code when it's important to not leave unmonitored code executing.

Describe the solution you'd like

Declare a new DisposableAsyncLazy<T> : IDisposable, IAsyncDisposable class.
Its disposal would perform the steps above. Dispose() would block on completion of the value factory and disposal where applicable. DisposeAsync() would return a ValueTask that completes when those same steps have completed.

The T would not be required to implement any interface, but upon disposal of the DisposableAsyncLazy<T>, if the instance of T implements IAsyncDisposable (either BCL or vs-threading variety) then the value will be disposed if created. Otherwise if the instance of T implements IDisposable that interface will be used to dispose of the value. Note that T itself may not derive from these interfaces but if the concrete type that may derive from T does, we should dispose of the value as described.

When T does not implement any recognized disposable interface, DisposableAsyncLazy<T> may still be an appropriate choice when it is desirable

The value factory will take a CancellationToken which is only canceled upon disposal of the DisposableAsyncLazy<T> instance.
Although the GetValue and GetValueAsync methods will also take a CancellationToken, these will not propagate in any way to the value factory. This way the value factory can honor the CancellationToken given to it even after side effects may have been applied that would be unsafe to cancel afterward because it can assume cancellation means the overall object graph is being disposed of (e.g. shutdown).

The value factory will never be invoked more than once.

Describe alternatives you've considered

Add disposable functionality to AsyncLazy<T> itself. This would mean implementing IDisposable and/or IAsyncDisposable on the existing class, leading to a great many existing users getting flagged for not disposing of a value that may not need to be disposed of. Unfortunately, we cannot make AsyncLazy<T> only implement IDisposable when T itself implements it.

We considered making the value factory cancellable multiple times (like #952 proposes) but considered that that would tempt the value factory to ignore the token after side effects have been applied to avoid repeat execution, but ignoring cancellation can slow down app shutdown. When value disposal is a requirement then, we want to focus on just that being the cancellation cause and therefore always something the factory should honor.

Additional context

This design was hashed out with @matteo-prosperi and @SealSlicer.
We also discussed #952 and decided that the two styles of value factory cancellation would be best kept distinct across the two classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants