During switching to the new .NET Core 3's IAsynsDisposable, I've stumbled upon the following problem.
The core of the problem: if DisposeAsync throws an exception, this exception hides any exceptions thrown inside await using-block.
class Program
{
static async Task Main()
{
try
{
await using (var d = new D())
{
throw new ArgumentException("I'm inside using");
}
}
catch (Exception e)
{
Console.WriteLine(e.Message); // prints I'm inside dispose
}
}
}
class D : IAsyncDisposable
{
public async ValueTask DisposeAsync()
{
await Task.Delay(1);
throw new Exception("I'm inside dispose");
}
}
What is getting caught is the DisposeAsync-exception if it's thrown, and the exception from inside await using only if DisposeAsync doesn't throw.
I would however prefer it other way round: getting the exception from await using block if possible, and DisposeAsync-exception only if the await using block finished successfully.
Rationale: Imagine that my class D works with some network resources and subscribes for some notifications remote. The code inside await using can do something wrong and fail the communication channel, after that the code in Dispose which tries to gracefully close the communication (e. g., unsubscribe from the notifications) would fail, too. But the first exception gives me the real information about the problem, and the second one is just a secondary problem.
In the other case when the main part ran through and the disposal failed, the real problem is inside DisposeAsync, so the exception from DisposeAsync is the relevant one. This means that just suppressing all exceptions inside DisposeAsync shouldn't be a good idea.
I know that there is the same problem with non-async case: exception in finally overrides the exception in try, that's why it's not recommended to throw in Dispose(). But with network-accessing classes suppressing exceptions in closing methods doesn't look good at all.
It's possible to work around the problem with the following helper:
static class AsyncTools
{
public static async Task UsingAsync<T>(this T disposable, Func<T, Task> task)
where T : IAsyncDisposable
{
bool trySucceeded = false;
try
{
await task(disposable);
trySucceeded = true;
}
finally
{
if (trySucceeded)
await disposable.DisposeAsync();
else // must suppress exceptions
try { await disposable.DisposeAsync(); } catch { }
}
}
}
and use it like
await new D().UsingAsync(d =>
{
throw new ArgumentException("I'm inside using");
});
which is kind of ugly (and disallows things like early returns inside the using block).
Is there a good, canonical solution, with await using if possible? My search in internet didn't find even discussing this problem.
Closemethod for this very reason. It's probably wise to do the same:CloseAsyncattempts to close things down nicely and throws on failure.DisposeAsyncjust does its best, and fails silently. – SoniaCloseAsyncmeans that I need to take extra precautions to get it running. If I just put it at the end ofusing-block, it will be skipped on early returns etc. (this is what we would want to happen) and exceptions (this is what we would want to happen). But the idea looks promising. – AggriDisposehas always been "Things might have gone wrong: just do your best to improve the situation, but don't make it worse", and I don't see whyAsyncDisposeshould be any different. – SoniaDisposeAsyncdo its best to tidy up but not throw is the right thing to do. You were talking about intentional early returns, where an intentional early return might mistakenly bypass a call toCloseAsync: those are the ones forbidden by many coding standards. – Soniausing) to forbidding them doesn't seem to be a good choice. – AggriDisposeAsyncshould do its best to tidy up without throwing though: that's howDisposeworks. If you need aCloseAsyncmethod which does throw on failure, write one. – SoniaDisposeAsync? Stop usingawait usingand code everything with try/finally? Stop usingIAsyncDisposableand code everything with try/finally? – Aggri