9

Do I need to call dispose in the finally block for SqlTransaction? Pretend the developer didnt use USING anywhere, and just try/catch.

SqlTransaction sqlTrans = con.BeginTransaction();

try
{
     //Do Work
sqlTrans.Commit()
}
catch (Exception ex)
        {

           sqlTrans.Rollback();
        }

 finally
        {
            sqlTrans.Dispose();
            con.Dispose();
        }
Mike Flynn
  • 21,905
  • 50
  • 167
  • 308
  • doesn't hurt to have it. – Brian Mar 01 '12 at 22:41
  • 1
    If you're not using a `using` around `sqlTrans` then it certainly doesn't hurt to explicitly call `Dispose()` on it. – Cᴏʀʏ Mar 01 '12 at 22:42
  • @Cory Is it required to check whether sqlTrans is already disposed before calling sqlTrans.Dispose() ? – LCJ Jan 18 '13 at 11:44
  • 1
    @Lijo: No, the `Dispose()` method should check whether it has been disposed and do nothing if it has. – Cᴏʀʏ Jan 18 '13 at 14:25
  • @Cory Answers in following question says the opposite http://stackoverflow.com/questions/14438736/how-to-check-state-before-disposing-sqltransaction/14438880#14438880 – LCJ Jan 21 '13 at 12:44

3 Answers3

18

Do I need to use try-finally or the using-statement to dispose the SqlTransaction?

It does not hurt to have it. This is true for every class implementing IDisposable, otherwise it would not implement this interface.

But normally the garbage collector deals with unreferenced objects(it doesn't mean that the GC calls dispose, which isn't true), so you need it only for unmanaged resources. But because i also don't want to call dispose on every other variable or use the using-statement everywhere, it it's always worth to look into the actual implementation of the class' Dispose method.

SqlTransaction.Dispose:

protected override void Dispose(bool disposing)
{
    if (disposing)
    {
        SNIHandle target = null;
        RuntimeHelpers.PrepareConstrainedRegions();
        try
        {
            target = SqlInternalConnection.GetBestEffortCleanupTarget(this._connection);
            if (!this.IsZombied && !this.IsYukonPartialZombie)
            {
                this._internalTransaction.Dispose();
            }
        }
        catch (OutOfMemoryException e)
        {
            this._connection.Abort(e);
            throw;
        }
        catch (StackOverflowException e2)
        {
            this._connection.Abort(e2);
            throw;
        }
        catch (ThreadAbortException e3)
        {
            this._connection.Abort(e3);
            SqlInternalConnection.BestEffortCleanup(target);
            throw;
        }
    }
    base.Dispose(disposing);
}
        

Without understanding all(or anything) what is happening here i can say that this is more than a simple base.Dispose(disposing). So it might be a good idea to ensure that a SqlTransaction gets disposed.

But because SqlConnection.BeginTransaction creates the transaction it could also be a good idea to reflect this also:

public SqlTransaction BeginTransaction(IsolationLevel iso, string transactionName)
{
    SqlStatistics statistics = null;
    string a = ADP.IsEmpty(transactionName) ? "None" : transactionName;
    IntPtr intPtr;
    Bid.ScopeEnter(out intPtr, "<sc.SqlConnection.BeginTransaction|API> %d#, iso=%d{ds.IsolationLevel}, transactionName='%ls'\n", this.ObjectID, (int)iso, a);
    SqlTransaction result;
    try
    {
        statistics = SqlStatistics.StartTimer(this.Statistics);
        SqlTransaction sqlTransaction = this.GetOpenConnection().BeginSqlTransaction(iso, transactionName);
        GC.KeepAlive(this);
        result = sqlTransaction;
    }
    finally
    {
        Bid.ScopeLeave(ref intPtr);
        SqlStatistics.StopTimer(statistics);
    }
    return result;
}

As you can see. The GC will also keep the Connection alive when a Transaction is created. It also doesn't hold a reference to the transaction since it only returns it. Hence it might not be disposed even when the connection is already disposed. Another argument to dispose the transaction.

You might also have a look at the TransactionScope class which is more fail-safe than BeginTransaction. Have a look at this question for more informations.

Tim Schmelter
  • 411,418
  • 61
  • 614
  • 859
  • You also may want to look at BeginSqlTransaction() and Connection.Dispose() code. SqlTransaction seems to be a wrpapper around its InternalTransaction that is cleared in Connection.Dispose(). – the_joric Mar 01 '12 at 23:34
  • 1
    I respectfully disagree with the comment `This is true for every classs implementing IDisposable`. What about TableCell control and other HTML controls. It is advised NOT to apply 'using' on them. – LCJ Jan 18 '13 at 11:37
  • 1
    @Lijo: Point taken, that was too general. You only need to override the implementation or use the `using`-statement if you need to dispose some unmanaged resources like a database connection. A control implements `IDposable` because the control (f.e. a `UserControl`) might contain unmanaged resources and it will be disposed at the end of the life-cycle ( page recursively into all child-controls ). – Tim Schmelter Jan 18 '13 at 11:59
  • 1
    Wouldn't the `SqlConnection` dispose the `SqlTransaction` once the `SqlConnection` is disposed? – zig Apr 24 '17 at 08:28
  • @zig: read my answer: _"The GC will also keep the Connection alive when a Transaction is created. It also don't hold a reference to the transaction since it only returns it. Hence it might **not be disposed even when the connection is already disposed**. Another argument to dispose the transaction."_ – Tim Schmelter Apr 24 '17 at 08:41
  • @Tim, I'm pretty sure the connection holds reference to the created Transaction object, and I use that reference in my project. http://stackoverflow.com/questions/417024/can-i-get-a-reference-to-a-pending-transaction-from-a-sqlconnection-object – zig Apr 24 '17 at 09:38
  • Can the wording "normally the garbage collector would deal with it if the object is not referenced anymore" be changed to avoid misleading people to think that the GC calls Dispose, which [it does not](https://stackoverflow.com/questions/45036/will-the-garbage-collector-call-idisposable-dispose-for-me) – Richardissimo Mar 23 '18 at 06:52
  • @Richardissimo: Done :) – Tim Schmelter May 25 '21 at 19:13
8

In the general case, every IDisposable object you construct or obtain, and own, you have to dispose of.

In the specific case, there are some exceptions, but SqlTransaction is not one of them.

As per the the documentation of SqlTransaction.Dispose:

Releases the unmanaged resources used by the DbTransaction and optionally releases the managed resources.

(my emphasis)

Since the documentation does not state that those unmanaged resources are released when you issue a commit or a rollback, you will need to dispose of this object.

Lasse V. Karlsen
  • 350,178
  • 94
  • 582
  • 779
1

I think you can just close connection. I checked the code of BCL -- seems like connections takes care of its transaction -- no need to close it explicitly.

the_joric
  • 10,986
  • 3
  • 32
  • 53