Do I need to explicitly rollback transaction if the connection is dipsosed?

Question

I was reading about transactions and saw this example from Microsoft docs. The gist from the example:

using (SqlConnection connection = new SqlConnection(connectionString))
{
    connection.Open();
    SqlTransaction sqlTran = connection.BeginTransaction();
         
    try
    {
        // Execute some commands
        sqlTran.Commit();
    }
    catch (Exception ex)
    {   
        try
        {
            // Attempt to roll back the transaction.
            sqlTran.Rollback();
        }
        catch (Exception exRollback)
        {
            Console.WriteLine(exRollback.Message);
        }
    }
}

What I don't understand here is why do we need an explicit rollback? As far as I can see the using statement will close the connection and rollback the transaction that was not committed?

The answers for a question here are contradicting, one saying that transaction is not rolled back, while the other saying that Ado.Net connection pooling will fix it and transaction is rolled back before connection is returned to the pool (which makes sense actually).

P.S. I have actually tried to check it and it seems the disposing of the connection is enough for the transaction to be rolled back. But I guess there are some circumstances where that might not be enough

Given that gbn asnwers a *lot* of SQL Server questions and even explains that forgetting to dispose will only close the transaction *eventually* - don't use such code in the first place. Transactions are meant to be used with a `using` block. You *really* don't want a transaction to delay rolling back, especially if you want to use app locks. You may end up with deadlocks caused because one transaction or another remained active even though the connection was closed — Panagiotis Kanavos, Nov 25 '20 at 09:23
That example is, in a word, crap, and doesn't demonstrate best practices. In fact it's almost as if someone went out of their way to demonstrate a host of different things you should *not* do (not disposing a transaction, exception handler within a handler, catching generic `Exception`...) Fortunately anyone with a Github account can propose changes now. ...not me because I'm too lazy. — Jeroen Mostert, Nov 25 '20 at 09:25
Should I explicitly dispose a transaction then? (And that will make sure it is rolled back?) Nice that microsoft has "wrong" docs... — Ilya Chernomordik, Nov 25 '20 at 09:27
MSDocs for some reason seem to be allergic to `using` blocks. Despite it being in their own best practices. I've submitted multiple PR adding usings to lumps of their codes. — Liam, Nov 25 '20 at 09:30
The recommended pattern is to use `TransactionScope`, stick it in a `using` and use only `.Complete()`. Explicit rollback is unnecessary and harmful (because an error there "overwrites" the original exception). In some scenarios an explicit `Transaction` may be preferable or necessary, same rules apply -- use `using` and `.Commit()`. [This example](https://learn.microsoft.com/dotnet/api/system.transactions.transactionscope?view=net-5.0#examples) still has problems (not disposing `SqlCommand`s), but is a lot better than the one in the question. — Jeroen Mostert, Nov 25 '20 at 09:30
They're trying to put too much in each code snippet lately. The [BackgroundService article](https://learn.microsoft.com/en-us/aspnet/core/fundamentals/host/hosted-services?view=aspnetcore-5.0&tabs=visual-studio) tries to demonstrate timers and queues in the same article that's supposed to explain what a hosted service is, or how to use scoped services. The "examples" end up drowning out the actual docs. — Panagiotis Kanavos, Nov 25 '20 at 09:33

Panagiotis Kanavos · Accepted Answer · 2020-11-25T09:37:00.500

That's an unfortunate example. Perhaps they wanted to illustrate how to handle errors with try/catch before rolling back ?

The safe way to use transactions is to use a using block. You really, really don't want a transaction to remain active for any longer than necessary, as it accumulates locks which end up blocking other connections or can even cause deadlocks.

In fact, the easy way to demonstrate a deadlock is to read/modify the same data in a different order using transactions.

The following snipped would rollback the transaction immediately in case of error:

using (SqlConnection connection = new SqlConnection(connectionString))
{
    connection.Open();
    //Do stuff that doesn't need a transaction,
    //eg read some data
    using(var sqlTran = connection.BeginTransaction())
    {
        //Do stuff that does need a transaction
        //Eg multiple UPDATEs or INSERTs
        sqlTran.Commit();
    }
}

Knowing that you're trying to work with explicit app locks in the database, you should be allergic to anything that could affect or delay a transaction's lifetime. Unlike regular row locks, app locks are very few and the chances of collision very high.

I have actually tried to check it and it seems the disposing of the connection is enough for the transaction to be rolled back. I guess there are some cases where this is not true? — Ilya Chernomordik, Nov 25 '20 at 09:42
Yes, that seems reasonable enough (even though I don't use distributed transactions). It's also best practice to dispose explicitly all of the owned objects that has IDisposable interface anyway I suppose, so it's a good idea to do it. Fortunately the new using syntax makes it even easier, without "nesting" — Ilya Chernomordik, Nov 25 '20 at 09:56
@IlyaChernomordik the question you linked to explains how things can be delayed, or a double exception can lead to missed rollbacks. It's just a lot easier and cleaner to use transactions safely though — Panagiotis Kanavos, Nov 25 '20 at 09:57

Do I need to explicitly rollback transaction if the connection is dipsosed?

1 Answers1