Potentially serious memory leak with regards to AdoTransaction Rollback

Description

I've been fixing this issue since NHibernate 2.1, and I finally decided to report it. I run into it using windbg to discover and production server memory leak. As I found lots of AdoTransaction objets retained in memory, I checked its code. Here is the summary of the problem and a possible solution.

PROBLEM
AdoTransaction.cs, when rolling back, makes:

if (!commitFailed)
{
try
{
* trans.Rollback();
* log.Debug("IDbTransaction RolledBack");
rolledBack = true;
* Dispose();
* }

As a consequence to that, when trans.Rollback (real connection rollback to db), fails... (many states in the connection can cause that... not only closed connection, but also logical (frequent) stuff), Dispose is ommited. As disposed is in charge of:

Dispose

if (isDisposing)
{
if (trans != null)
{
trans.Dispose();
trans = null;
log.Debug("IDbTransaction disposed.");
}

trans.Dispose is never made (trans is MS ADO.DB transaction). If your force that through test, you will see handles and memory raise (not quickly, but under stress it finally matters).

*WORKAROUND
*
My solution maybe was not the best, but it worked. You certainly can do better =)

I added in ConnectionManager, method AfterTransaction, the lines:

if (transaction != null)
transaction.Dispose();

before transaction = null;

what lead to change adoTransaction.Rollback moving AfterTransactionCompletion(false) before Dispose and duplicating it into catch in:

if (!commitFailed)
{
try
{
trans.Rollback();
log.Debug("IDbTransaction RolledBack");
rolledBack = true;

  • AfterTransactionCompletion(false);
    * Dispose();
    }
    catch (HibernateException e)
    {
    log.Error("Rollback failed", e);
    // Don't wrap HibernateExceptions
    throw;
    }
    catch (Exception e)
    {

  • AfterTransactionCompletion(false);

  • log.Error("Rollback failed", e);
    throw new TransactionException("Rollback failed with SQL Exception", e);
    }
    finally
    {
    CloseIfRequired();
    }

I do not remember now why releasing at connectionManager made more sense than moving dispose to the finally part in Rollback (maybe that covered non rollback leaky cases... it was 4 years ago and I do not remember now).

Hope this helps, Pablo.

Environment

None

Activity

Show:

Pablo De Grande May 20, 2016 at 6:55 AM

Something when wrong with the richtext formatting control.... If you cannot read through the code, let me know. Byebue....

Duplicate

Details

Assignee

Reporter

Labels

Components

Affects versions

Priority

Who's Looking?

Open Who's Looking?

Created May 20, 2016 at 6:54 AM
Updated June 5, 2016 at 2:15 PM
Resolved June 5, 2016 at 2:15 PM
Who's Looking?

Flag notifications