Handling exception in Dispose()

Question

I have a UnitOfWork class that commits or rolls back transactions depending on errors in the block.

class UnitOfWork: IDispose
{
        public void Dispose()
        {
                if (Marshal.GetExceptionCode() == 0 
                   || Marshal.GetExceptionPointers() == IntPtr.Zero)
                {
                       // Commit
                }
                else
                {
                      // Rollback
                }
            }
        } 
}

This class is used in the following manner:

try
{
    using (var uow = new UnitOfWork())
    {
        // Do something here that causes an exception
    }
}
catch
{
    using (var uow = new UnitOfWork())
    {
        // Log exception
    }
}

I have gone through this article: Detecting a Dispose() from an exception inside using block

In the above question, Steven (one of the people who replied) mentions that if I detect an exception inside Dispose() for the "using" block from the "try" section, I can not create another UnitOfWork class in the "catch" block and log errors to the database (since the "Marshal" code in Dispose() will continue to say there is an exception), meaning the ExceptionCode and ExceptionPointers will "float" (?) from the try block to the catch block.

However, in my case, this works correctly - ie. if there is an exception in the "using (var uow = new UOW)" block in the "try" section, it correctly catches the exception and then in the catch block the "if" condition passes, causing a commit to happen and allowing me to log exceptions.

It boils down to me having an incomplete understanding of how Marshal.GetExceptionPointers() work. Can someone please confirm if my usage pattern will work correctly (so far all tests seem to show that it works correctly) and what exactly is Marshal.GetExceptionPointers() doing?

Cheng Chen · Answer 1 · 2016-05-24T10:07:12.080

1

IDisposable.Dispose is designed to release resources, putting commit/rollback actions in it is a bad idea. The caller should know how to handle the exception and how to rollback.

using (var unitOfWork = new UnitOfWork())
{
    try
    {
        Prepare();
        Step1();
        Step2();
        unitOfWork.Commit();
    }
    catch(PrepareException e)
    {
        //no necessary to rollback, just log it
    }
    catch(FirstStepException e1)
    {
        //rollback step1
    }
    catch(SecondStepException e2)
    {
        //rollback step1 and step2
    }
}

edited May 24 '16 at 10:07

answered May 24 '16 at 09:52

Cheng Chen

42,509
16
113
174

While putting Commit in Dispose is certainly bad idea, putting Rollback there is not, and is actually done in many places, including standard DbTransaction, TransactionScope etc. – Evk May 24 '16 at 09:57
1

@Evk The problem is sometimes there is no global rollback solution. Only the caller knows what is done and what needs to be rollback. See my edit. – Cheng Chen May 24 '16 at 10:05
But what other options you have (if we talk about abstract Transaction and it's Dispose method)? You cannot leave partial changes applied so in Transaction.Dispose you have to either commit or rollback anyway. – Evk May 24 '16 at 10:12
I am not sure why it is a bad idea? Dispose is releasing resources, but the UnitOfWork class has a reference to a SqlDAO class (which encapsulates a SqlConnection class). I explicitly rollback or commit a transaction before the dispose on sqlDao is fired. What problem do you foresee? – Taha Ahmad May 25 '16 at 05:00

Handling exception in Dispose()

1 Answers1