Allow try-catch-finally semantics with the using-construct

Topics: C# Language Design
Jul 9 at 9:52 AM
Edited Jul 10 at 7:22 AM
By adding a new interface to the language (let's call it IExceptionDisposable in this post, but let somebody who knows something about naming do the actual naming) that extends IDisposable and looks something like this:
public interface IExceptionDisposable : IDisposable 
{
    void OnException(Exception e);
}
This would allow modification to the using-statement where this new interface was used to this:
IExceptionDisposable disp;
try
{
    disp = MakeSomethingDisposable();
}
fault (Exception e)
{
    if (disp != null) { disp.OnException(e); }
}
finally
{
    if (disp != null) { disp.Dispose(); }
}
The new rewrite would only be valid in places where the compiler could prove at compile-time that the using statement contained an element of the new interface-type (ie, everything that returned an regular IDisposable would still get the old rewrite, even if at runtime the disposable turned out to be a IExceptionDisposable).

This would allow for stuff like simple logger that could log exceptions (without the use of AOP):
public class UnitOfWorkLogger : IExceptionDisposable
{
    public UnitOfWorkLogger(string name) 
    {
        Console.WriteLine("Start of " + name);
    }

    public void OnException(Exception e)
    {
        Console.WriteLine("Error: " + e.ToString());
    }

    public void Dispose()
    {
        Console.WriteLine("Done");
    }
}
Which could simply be used as following:
using(new UnitOfWorkLogger("test"))
{
    throw new Exception("I will be logged :)");
}
The way this is achieved today is with delegates, for instance I have like this:
return Log.UnitOfWork("test", () => {
    throw new Exception("I will be logged :)");
    return ""; // this is to make sure that the delegate is infered to be Action<string>
});
This is awkward, and requires me to have 2 return statements.

[Edit]
Changed from catch to fault, so that the exception is never actually caught.
Jul 9 at 2:55 PM
Another problem I just found with the delegate ways is if your function have out parameters.

Take the following example:
IEnumerable<string> GetDbRecords(int skip, int take, out int total)
{
    using(new UnitOfWorkLogger("get db records")) 
    {
        total = _db.Records.Count();
        return _db.Records.Skip(skip).Take(take).ToList();
    }
}
Whereas today I have to do something like this:
IEnumerable<string> GetDbRecords(int skip, int take, out int total)
{
    var temp = Log.UnitOfWork("get db records", () => {
        return Tuple.Create(
            _db.Records.Count(),
            _db.Records.Skip(skip).Take(take).ToList()
        );
    }
    total = temp.Item1;
    return temp.Item2;
}
This is obviously a lot less practical, and it's harder to see what's going on.
Jul 9 at 3:36 PM
Edited Jul 9 at 3:38 PM
Hey! + 1 for this idea.

I will rename OnException for HandleException and return bool. Then change the pattern to
if(!disp.HandleException(e))
   throw;
Jul 9 at 4:53 PM
You're seeking something I've been wanting, though I'd handle it a bit differently. The first key would be to extend finally to allow the syntax finally(Exception ex) { code here }, with the semantics that ex would hold any exception that occurred in the try block, or null if none occurred; additionally, I would add the interfaces:
interface IDisposableExOnly { void Dispose(Exception Ex); }
interface IDisposableEx : IDisposable, IDisposableExOnly { }
and have using blocks start by testing whether the argument implements IDisposableExOnly; if it does, then using would behave as:
try
{
  code in using block
}
finally(Exception ex)
{
   if (guardVariable != null)
    ((IDisposableExOnly)guardVariable).Dispose(ex);
}
One advantage of this pattern compared with adding a separate method for use in case of a try-block exception is that there are many situations where the main-block-succeeded and main-block-failed cases should do many of the same things, but differ slightly. For example, a common pattern might be:
void IDisposableExOnly.Dispose(Exception ex)
{
  try
  {
    do cleanup        
  }
  finally (Exception ex2)
  {
    if (ex != null && ex2 != null)
      throw new DoubleFaultException("Cleanup fault occurred with exception pending", ex, ex2);
  }
}
The cleanup code wouldn't care about any exception which occurred in the main using block if the cleanup itself was successful (it would simply bubble up to the caller), but if cleanup failed the cleanup code would ensure that both exceptions were made available to the caller--something which is at present not possible.

The reason I suggest segregating IDisposableExOnly is to allow classes to either implement IDisposableEx, in which case old compilers would handle them the old way, and new ones the new way, or could implement IDisposableExOnly without implementing IDisposable, in case they should only be usable by new compilers. Such usage might be appropriate for a transaction-wrapping class which would throw an exception if code exited without having performed a Commit or Rollback and no exception was already pending. Such a class could only provide proper semantics if it was informed of whether the using block was exited via exception.

Note, by the way, that in no case would the new finally or using block cause exceptions to be caught. Catching and rethrowing an exception--even if one uses throw; rather than throw Ex;--is not quite the same thing as not catching it. Although C# doesn't presently make it possible to exploit this, code in a .NET try block actually has three ways to process an exception: in a filter handler which runs before inner finally blocks execute, between inner try blocks and outer filter handlers, or after inner try blocks and outer filter handlers. Although C# generally only exposes the second approach (iterators can achieve the semantics of the third approach), the finally and using blocks as described would use the third approach, thus playing nicely with any code written in other languages which would expect to use the first.
Jul 10 at 7:28 AM
@Olmo I disagree that the using-statement should be able to dismiss exceptions. If you want the ability to catch exceptions I think you should have to be explicit about it, otherwise you might just shoot yourself in the foot.

@supercat I designed the feature so that no changes would have to be made to the underlying runtime, only the compiler. This is also the main reason for adding a new handler-method, instead of changing the present one. The problem with changing the present handler, like you've done, is that (as far as I can understand) you would either have to change IL, or you end up having to keep state of whether or not you've called into the Dispose method already, given that a try-fault-finally would call into both fault and finally upon an exception, no? Also, as you might see in my original post, I've changed it to use fault instead of catch. Fault is obviously not a C# construct, but it works in IL, and allows us to not catch the exception while still knowing there is one, thus I'm not rethrowing.

Out of curiosity though, how does throw; change the exception?
Jul 10 at 4:02 PM
Most of the time when I want to re-throw an exception I use throw;, otherwise the original callstack is overwritten and it's much harder to find the original cause of the bug.

I think one important use car of this feature is logging, but many times you also want the exception to continue bubbling till the global handler in the user interface. Having a way to still call throw; is a must.

What's your example for shouting yourself in the foot?
Jul 10 at 5:53 PM
Edited Jul 10 at 5:53 PM
@Olmo My point was that it should always rethrow, not give you the option not to. I don't want to do using(new SomeClassIDidNotMake()) and then suddenly not get exceptions. That would be totally unexpected (and a good way to shoot yourself in the foot, without even knowing it). You might have hundreds of exceptions just swallowed, without knowing about it.
Jul 10 at 7:47 PM
Alxandr wrote:
@supercat I designed the feature so that no changes would have to be made to the underlying runtime, only the compiler. This is also the main reason for adding a new handler-method, instead of changing the present one. The problem with changing the present handler, like you've done, is that (as far as I can understand) you would either have to change IL, or you end up having to keep state of whether or not you've called into the Dispose method already, given that a try-fault-finally would call into both fault and finally upon an exception, no? Also, as you might see in my original post, I've changed it to use fault instead of catch. Fault is obviously not a C# construct, but it works in IL, and allows us to not catch the exception while still knowing there is one, thus I'm not rethrowing.
My intention was that there would be one call to Dispose, which would have the Exception parameter be null if the main body succeeded, or non-null if it didn't. I wouldn't have the compiler implement it with fault blocks, actually, but rather with an exception filter, equivalent to the VB code:
Dim PendingException As Exception = Nothing
Try
  .. Do Something
  PendingException = Nothing ' See note below
Catch Ex2 As Exception When CopyFirstArgumentToSecondAndReturnFalse(Ex2, PendingException)
  Throw ' Never executes
Finally
  If GuardedObject IsNot Nothing Then GuardedObject.Dispose(PendingException)
End Try
Note that an odd situation may arise if an exception is thrown from a method deeply nested within the "do something" but the closest handler is outside the try block (causing the exception to get copied to PendingException) but another exception occurs while unwinding that one, and the second exception is caught within the try/finally block. In that case, the code within the try block might complete normally despite an exception having been thrown which was not handled within that block. If PendingException isn't null at the end of the Try block, that would imply that the exception represented thereby was overwritten by an exception thrown in a Finally block, and PendingException probably contains useful information, but I can't think of anything "non-surprising" the compiler could do with it.
Out of curiosity though, how does throw; change the exception?
For one thing, given
try
{
   method1(firstParameters);
   method1(secondParameters);
}
catch (Exception ex)
{
   if (someCondition)
     fixProblem();
   else
     throw;
}
If an exception occurs in one of the calls to method1 and a valid .pdb file is available, the stack trace will report the line number of the rethrow, rather than the line number of the call which threw the exception. Further, a catch-and-rethrow will cause all inner finally blocks to run before any outer filter blocks, but if an exception isn't caught the outer filter blocks will be able to see it before inner finally blocks.
Jul 10 at 8:20 PM
Hmm. Well. I wont argue much either way here. I prefer two methods IMHO, and think it's cleaner, but I would be happy with any of these, as long as the Dispose method (or whatever it's called) has no way to intercept the exception, it can just observe it.
Jul 10 at 8:52 PM
Edited Jul 10 at 8:54 PM
Oh! Now I get it. You're right that letting the exception flow is the most common possibility. Not sure if the only allowed.

I've a use case for stopping the exception:

In many WCF services we do something like:
public int Sum(int a, int b)
{
  return Return(()=>
       Calculator.Sum(a,b)); 
}
The Return method returns whatever value the lambda returns, but has all the boilerplate for logging, profiling, exception handling, etc... Probably we can do this combining many WCF filters but we find this easier to understand.

One thing that this method does is capture all the exceptions, log them, and convert them to FaultException.

In this case we can still implement this pattern with your solution because we will throw the new FaultException in OnException method... but dunno if one day I will like to swallow some exception types....

Mmmm... actually you right. If the IExceptionDisposable will shallow the exceptions will cause problems to the definitely assignment/all code path returns a value analysis.

So, I just vote for your initial solution. No changes needed. I will be able to write something like:
public int Sum(int a, int b)
{
  using(ServiceBilerplate())
      return Calculator.Sum(a,b); 
}
Jul 11 at 12:16 AM
Alxandr wrote:
Hmm. Well. I wont argue much either way here. I prefer two methods IMHO, and think it's cleaner, but I would be happy with any of these, as long as the Dispose method (or whatever it's called) has no way to intercept the exception, it can just observe it.
I think there needs to be a distinction between "an IDisposableEx-aware block finished with no exceptions" and "a block which isn't aware of IDisposableEx has exited with unknown exception status". Disposing a transaction scope when a transaction has been started but neither committed nor rolled back should roll back the transaction, but it should also be regarded a usage error, and should throw an exception if none was already pending. Since the most common reason for a transaction scope to be exited without a commit or explicit rollback would be that an exception occurred while it was open, it would be very obnoxious for the parameterless Dispose to throw an exception in such a case (since it would overwrite the earlier exception), but Dispose(null) should throw an exception since it would know that none was pending.

Also, I would favor using one method (for IDisposableEx-aware using block) because in many cases there will be little difference between the exception-pending and no-exception-pending behaviors. If the only time the two behaviors should differ is when an exception occurs during cleanup, the most reasonable way to implement the method would be to have the "success" code chain to a common method which was used for both success and failure.
Jul 11 at 1:32 PM
Ah, yes, but you're misunderstanding something. On exception, both methods will be called. So take your example of transaction. If Dispose is called, and it's not already either committed or rolled back, it can throw. Cause if the using block exits with a fault, OnException is triggered first, then Dispose. So in the normal case, you just implement IDisposabe (the old one), and only where you need to differ at all, you add an OnException method (and mark it with the new interface). This would allow for easy upgrading of legacy code, and it's almost compatible with it. And an IDisposableEx could still be used in legacy code, the OnException method would just never get called.

Though, I just explained how you could implement transactions that threw if not committed, I would not do it though. I would stick to the Dispose()-methods shall not throw guideline. However, OnException should be allowed to throw, cause it can set the inner exception to whatever triggered it.
Jul 11 at 4:40 PM
Edited Jul 11 at 4:42 PM
Alxandr wrote:
Ah, yes, but you're misunderstanding something. On exception, both methods will be called. So take your example of transaction. If Dispose is called, and it's not already either committed or rolled back, it can throw. Cause if the using block exits with a fault, OnException is triggered first, then Dispose. So in the normal case, you just implement IDisposabe (the old one), and only where you need to differ at all, you add an OnException method (and mark it with the new interface). This would allow for easy upgrading of legacy code, and it's almost compatible with it. And an IDisposableEx could still be used in legacy code, the OnException method would just never get called.
The Dispose method should not throw on an abandoned-transaction exception if called by a code which wouldn't know how to indicate that an exception was pending, since the most common situation where exceptions would be abandoned would be precisely the situation where Dispose shouldn't throw (because throwing an exception in Dispose would cause another exception to get lost).

The ugliest aspect I can see with the IDisposableEx:IDisposable,IDisposableExOnly, design is that it does not have any particularly clean way of wrapping an object that wraps an IDisposable that might also implement IDisposableEx, especially if one would want a single wrapper method to handle all three disposal cases (disposed after success; disposed for exception; dispose for unspecified reason). Perhaps defining a CleanupStatus structure type with a single private field of type Exception and properties to query the exception cause would help. Passing that around would be as efficient as passing an Exception, but rather than using null to indicate that there was no exception, it could interpret a null field value as "status unknown" and use a private static singleton Exception instance as a flag for "known success". Adding yet another type of which the compiler would have to be aware might be a little icky, but it would let code implement IDisposable.Dispose as a simple call to Dispose(CleanupStatus.Unknown).

BTW, another approach might be to have a static CleanupStatus class which the compiler would keep informed regarding disposal status, and which code could query to find out what's going on. I haven't figured out a way for that to really work, though. If a the cleanup from a using is something like void Dispose() { try {resource1.Dispose();} finally {resource2.Dispose(); } then resource2,Dispose() should regard the situation where the original try block throws an exception but resource1.Dispose() succeeds as being the "exception" case rather than a "normal" case, but there would be no mechanism I can see--other than an exception-status parameter, via which resource2.Dispose() could know that.
Though, I just explained how you could implement transactions that threw if not committed, I would not do it though. I would stick to the Dispose()-methods shall not throw guideline. However, OnException should be allowed to throw, cause it can set the inner exception to whatever triggered it.
If the transaction was abandoned because of an exception, the fact that that earlier exception gets thrown out of the using block will let the caller know the transaction was likely abandoned. My point is that Dispose() should throw an exception when a transaction is abandoned for some reason other than stack unwinding from some other exception.
Jul 12 at 12:25 AM
I still favor the two-function approach, due to the fact that the changes involved are so minimal. However, as stated earlier, how exactly it's implemented isn't too important to me, as long as I get the ability to observe exceptions using a construct equal or similar to using statements, without interrupting said exception. This could probably quite easily be done with AOP already, using a roslyn tree rewriter to find using statements that have a IDisposableEx (or similar) and rewrite them into try-catch-finally, however, as C# doesn't support fault-blocks, like IL does, I don't think it's possible to rewrite it into a try-fault-finally like I would have wanted to do. For that you would probably have to use something like Cecil to rewrite the IL after it's compiled.
Jul 14 at 9:26 AM
Edited Jul 14 at 9:27 AM
Mads has mentioned that C# support for fault blocks was in the process of being added last fall. I'm not sure what the current status is - if it were in use in the Roslyn code base, that wouldn't compile except on Roslyn, and it's not on the language status wiki page. There's a code path that can emit faults, but it's only used in yield iterators.
Jul 14 at 10:03 AM
Yeah. As far as I know, generators have always emmited fault-blocks. Though, I'd rather have this new IDisposable feature, then fault blocks (if I had to choose one) ^^