Does SqlTransaction need to have Dispose called?

Question

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();
        }

Hypogenous

doesn't hurt to have it. – Wentletrap

If you're not using a using around sqlTrans then it certainly doesn't hurt to explicitly call Dispose() on it. – Tonality

@Cory Is it required to check whether sqlTrans is already disposed before calling sqlTrans.Dispose() ? – Felic

@Lijo: No, the Dispose() method should check whether it has been disposed and do nothing if it has. – Tonality

@Cory Answers in following question says the opposite #14439236 – Felic

Answer 1

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.

Limonite

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(). – Stricken

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. – Felic

@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 ). – Limonite

Wouldn't the SqlConnection dispose the SqlTransaction once the SqlConnection is disposed? – Amaro

@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." – Limonite

@Tim, I'm pretty sure the connection holds reference to the created Transaction object, and I use that reference in my project. #417524 – Amaro

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 – Applejack

@Richardissimo: Done :) – Limonite

Answer 2

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.

Devora

Answer 3

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.

Stricken

More details like a code extract would help with the confidence. – Explanatory