Re: [xwiki-devs] Question about hibernate transaction handling in XWiki

15 Apr 2010


      On 04/15/2010 01:07 PM, Asiri Rathnayake wrote:
...
Hi Devs,
While working on a client project I came across an issue where an exception
is thrown with a message that reads something like:
"Caused by: org.hibernate.HibernateException: Failed to commit or rollback
transaction. Root cause []"
Upon further digging I found the following code in XWikiHibernateBaseStore:
<code>
/**
  * Ends a transaction
*
* @param context
* @param commit should we commit or not
*/
public void endTransaction(XWikiContext context, boolean commit)
{
    endTransaction(context, commit, false);
}
/**
* Ends a transaction
*
* @param context
* @param commit should we commit or not
* @param withTransaction
* @throws HibernateException
*/
public void endTransaction(XWikiContext context, boolean commit, boolean
withTransaction) throws HibernateException
{
    Session session = null;
    try {
    session = getSession(context);
    Transaction transaction = getTransaction(context);
    setSession(null, context);
    setTransaction(null, context);
if (transaction != null) {
        // We need to clean up our connection map first because the
connection will
        // be aggressively closed by hibernate 3.1 and more
        preCloseSession(session);
if (log.isDebugEnabled()) {
            log.debug("Releasing hibernate transaction " + transaction);
        }
        if (commit) {
            transaction.commit();
        } else {
            transaction.rollback();
        }
    }
    } catch (HibernateException e) {
    // Ensure the original cause will get printed.
    throw new HibernateException("Failed to commit or rollback transaction.
Root cause ["
        + getExceptionMessage(e) + "]", e);
    } finally {
    closeSession(session);
    }
}
</code>
The problem i see in this code is that if preCloseSession(session) or
transaction.commit() throws an exception, there is no attempt made to
rollback the transaction. Normally a transaction rollback attempt should be
made within the catch block.
If I understand the basics of transaction handling, this code is wrong. I
want to discuss this with you (other developers) because I'm not an expert
in this subject :)
Yes, not calling rollback when an exception occurs, is a critical 
mistake. Does anybody know why we're not doing this? This is really old 
code, from the first days of XWiki, so this is an important change, we 
need to properly understand why is done like this. Ludovic, any thoughts?

-- 
Sergiu Dumitriu
http://purl.org/net/sergiu/