When combining a using statement with a fluent api that can potentially throw, the lowered code will never call dispose correctly.
If I have the following class that exposes a fluent interface:
public class Wrapper : IDisposable
{
private bool _isAdded;
public Wrapper Add()
{
_isAdded = true;
return this;
}
public void Dispose() => Console.WriteLine("dispose called");
public Wrapper ThrowIfAdded() => _isAdded ? throw new Exception() : this;
}
and I call it with the following:
using var willNotDispose = new Wrapper().Add().ThrowIfAdded();
the lowered code will result in the Dispose call occurring after the fluent method chain is completed.
Wrapper willNotDispose = new Wrapper().Add().ThrowIfAdded();
try
{
}
finally
{
if (willNotDispose != null)
{
((IDisposable)willNotDispose).Dispose();
}
}
Alternatively, if the call to .ThrowIfAdded() is done outside of the initial using declaration,
using var willDispose = new Wrapper().Add();
willDispose.ThrowIfAdded();
the lowered code is generated as expected.
Wrapper willDispose = new Wrapper().Add();
try
{
willDispose.ThrowIfAdded();
}
finally
{
if (willDispose != null)
{
((IDisposable)willDispose).Dispose();
}
}
While I understand why this is occurring, it isn't desirable. Is there any way to coerce the former initialization to compile to the latter? Ideally, it would be an attribute or form of compiler hint that would result in:
Wrapper willDispose = default;
try
{
willDispose = new Wrapper().Add().ThrowIfAdded();
}
finally
{
if (willDispose != null)
{
((IDisposable)willDispose).Dispose();
}
}
which I would have expected the original example to compile to in the first place.
As pointed out in the comments, there is pre-existing guidance that when an exception is thrown in a constructor, it should be explicitly handled and the resources cleaned up.
This extends to
CA2000analysis that states:While a Fluent API throwing an exception is not explicitly either a constructor or nested constructor throwing an exception, it should be treated the same since the object will be created and mutated outside of the try/finally block.
As a result, any method that can throw must first call dispose before allowing the exception to propagate.
This correctly assures that in cases where
.Added()is called,.ThrowIfAdded()will dispose prior to throwing.If
.Added()is not called, the instance will be disposed at the end of the block as expected.