3

I don't think this question is a duplicate of "Proper way to deal with exceptions in DisposeAsync".

Let's say my class that implements IAsynsDisposable because it has a long-running background task, and DisposeAsync terminates that task. A familiar pattern might be the Completion property, e.g. ChannelReader<T>.Completion (despite ChannelReader doesn't implement IAsynsDisposable).

Is it considered a good practice to propagate the Completion task's exceptions outside DisposeAsync?

Here is a complete example that can be copied/pasted into a dotnet new console project. Note await this.Completion inside DisposeAsync:

try
{
    await using var service = new BackgroundService(TimeSpan.FromSeconds(2));
    await Task.Delay(TimeSpan.FromSeconds(3));
}
catch (Exception ex)
{
    Console.WriteLine(ex);
    Console.ReadLine();
}

class BackgroundService: IAsyncDisposable
{
    public Task Completion { get; }

    private CancellationTokenSource _diposalCts = new();

    public BackgroundService(TimeSpan timeSpan)
    {
        this.Completion = Run(timeSpan);
    }

    public async ValueTask DisposeAsync()
    {
        _diposalCts.Cancel();
        try
        {
            await this.Completion;
        }
        finally
        {
            _diposalCts.Dispose();
        }
    }

    private async Task Run(TimeSpan timeSpan)
    {
        try
        {
            await Task.Delay(timeSpan, _diposalCts.Token);
            throw new InvalidOperationException("Boo!");
        }
        catch (OperationCanceledException)
        {
        }
    }
}

Alternatively, I can observe service.Completion explicitly in the client code (and ignore its exceptions inside DiposeAsync to avoid them being potentially thrown twice), like below:

try
{
    await using var service = new BackgroundService(TimeSpan.FromSeconds(2));
    await Task.Delay(TimeSpan.FromSeconds(3));
    await service.Completion;
}
catch (Exception ex)
{
    Console.WriteLine(ex);
    Console.ReadLine();
}

class BackgroundService: IAsyncDisposable
{
    public Task Completion { get; }

    private CancellationTokenSource _diposalCts = new();

    public BackgroundService(TimeSpan timeSpan)
    {
        this.Completion = Run(timeSpan);
    }

    public async ValueTask DisposeAsync()
    {
        _diposalCts.Cancel();
        try
        {
            await this.Completion;
        }
        catch
        {
            // the client should observe this.Completion
        }
        finally
        {
            _diposalCts.Dispose();
        }
    }

    private async Task Run(TimeSpan timeSpan)
    {
        try
        {
            await Task.Delay(timeSpan, _diposalCts.Token);
            throw new InvalidOperationException("Boo!");
        }
        catch (OperationCanceledException)
        {
        }
    }
}

Is there a concensus about which option is better?

7
  • In first approach, you want wait for background task to complete, if using scope has finished, the background task will be cancelled if it won't finish by that time. In the second case, you will await background task. THose two approaches are very different functionally, so I would not recommend one over another, I'd say whatever works for you and what is your use-case. Commented Jan 18, 2022 at 11:00
  • @MichałTurczyn, by "very different", do you mean that if I expose service.Completion, then maybe I should not even implement IAsyncDisposable and let the client track the completion status and any errors? Tks for the discussion! :)
    – noseratio
    Commented Jan 18, 2022 at 11:28
  • 1
    I would say so. If client invokes background task, then yeah. If I were a client, i would implement Disposable pattern and complete (or not) background tasks that I invoked. Commented Jan 18, 2022 at 11:32
  • 1
    Yes, your question is close to the referenced one. IMHO, it should be up to you and your requirements. It could be incapsulated to have clearer code, have less control in logic. Or it could be processed in logic with less clarity but have more control. More text in the accepted answer stackoverflow.com/a/59185614/6344916.
    – PIoneer_2
    Commented Jan 18, 2022 at 12:12
  • 1
    @noseratio For me the same. It is more intuitive to receive the exception via the Completion. But the main drawback is that you can't enforce the consumer of your class to call await service.Completion; , which can lead to swallowing exception. The consumer might think that the operation has succeeded even though it's not. Commented Jan 21, 2022 at 9:25

1 Answer 1

1

For now, I've settled on a reusable helper class LongRunningAsyncDisposable (here's a gist, warning: barely tested yet), which allows:

  • to start a background task;
  • stop this task (via a cancellation token) by calling IAsyncDisposable.DisposeAsync at any time, in a thread-safe, concurrency-friendly way;
  • configure whether DisposeAsync should re-throw the task's exceptions (DisposeAsync will await the task's completion either way, before doing a cleanup);
  • observe the task's status, result and exceptions at any time via LongRunningAsyncDisposable.Completion property.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.