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:
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:
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).
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:
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
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:
before transaction = null;
what lead to change adoTransaction.Rollback moving AfterTransactionCompletion(false) before Dispose and duplicating it into catch in:
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.