Re: [Springframework-developer] Hibernate resource management issue

[Colin S. <col...@ex...>]

Things are simpler than they appear, and are actually as per my original=20
email. Look at this, from SessionFactoryUtils:
        public void beforeCompletion() throws=20
CleanupFailureDataAccessException {
            if (this.newSession) {
               =20
TransactionSynchronizationManager.unbindResource(this.sessionFactory);
                if (this.hibernateTransactionCompletion) {
                   =20
closeSessionIfNecessary(this.sessionHolder.getSession(),=20
this.sessionFactory);
                }
            }
        }

        public void afterCompletion(int status) {
            if (!this.hibernateTransactionCompletion) {
                Session session =3D this.sessionHolder.getSession();
                if (session instanceof SessionImplementor) {
                    ((SessionImplementor)=20
session).afterTransactionCompletion(status =3D=3D STATUS_COMMITTED);
                }
                if (this.newSession) {
                    closeSessionIfNecessary(session, this.sessionFactory)=
;
                }
            }
            this.sessionHolder.setSynchronizedWithTransaction(false);
        }

beforeCompletion is the only place that
  TransactionSynchronizationManager.unbindResource
gets called for the SessionHolder, and in this case, beforeCompletion=20
never gets called.

So unfortunately this _is_ a serious bug. Any exception during the=20
beforeCommit call (which calls flush) means that the SessionHolder never=20
gets unbound from the thread. Nasty!

Colin


j=FCrgen h=F6ller [werk3AT] wrote:

>Odd - if there's no TransactionManagerLookup, the Session should definit=
ely be closed in afterCompletion.
>=20
>Regarding beforeCompletion, the semantics indeed need to be clarified: I=
 guess it's appropriate to always invoke it, even on beforeCommit failure=
.
>=20
>Juergen
>=20
>
>________________________________
>
>Von: Colin Sampaleanu [mailto:col...@ex...]
>Gesendet: Do 25.03.2004 21:09
>An: j=FCrgen h=F6ller [werk3AT]
>Cc: spr...@li...
>Betreff: Re: [Springframework-developer] Hibernate resource management i=
ssue
>
>
>
>We're on slightly different pages though. In my case, I am actually not
>using TransactionManagerLookup... But in my case, because of the
>exception in the flush in beforeCommit(), beforeCompletion() never gets
>called (as it would normally). Now I do see that afterCompletion is also
>supposed to call closeSessionIfNecessary()  (I had actually missed this
>before), but in my case, it's not getting there. I haven't traced it in
>a debugger, which is what I will do now, as it seems to me it should get
>to that code, certainly I do have a
>   "Triggering afterCompletion synchronization"
>in the log which should happen right before afterCompletion() is called.
>
>The other question is whether beforeCompletion still shouldn't be called
>in any case even if beforeCommit fails. Ultimately, we are still before
>completion of the transaction, unless you meant it to be only called in
>the case of no failure.
>
>Colin
>
>
>j=FCrgen h=F6ller [werk3AT] wrote:
>
> =20
>
>>Colin,
>>
>>Thanks for tracking this down. It's actually a problem with SessionFact=
oryUtils' inner class SessionSynchronization: It assumes that beforeCompl=
etion is called in any case, even if beforeCommit has thrown an exception=
. However, this just applies if you specified a TransactionManagerLookup =
in the Hibernate configuration; else, afterCompletion will do the cleanup=
 - which will be called in any case.
>>
>>When you remove the Hibernate TransactionManagerLookup, you shouldn't f=
ace the issue - the bug doesn't have any effects then. Note that you don'=
t need that TransactionManagerLookup when using Spring's JtaTransactionMa=
nager, as Spring will properly apply cache callbacks anyway. A Transactio=
nManagerLookup just adds value when used with EJB CMT or manual JTA, for =
ultra-correct cache callbacks.
>>
>>So essentially, everything should be fine if using HibernateTransaction=
Manager, or JtaTransactionManager without a Hibernate TransactionManagerL=
ookup. This is clearly something to fix, but I guess we don't need to do =
an immediate 1.0.1 followup release; I'd like to gather further bug repor=
ts first. For the time being, let's suggest to remove the TransactionMana=
gerLookup from the Hibernate configuration.
>>
>>Juergen
>>
>>
>>________________________________
>>
>>Von: Colin Sampaleanu [mailto:col...@ex...]
>>Gesendet: Do 25.03.2004 20:32
>>An: spr...@li...; j=FCrgen h=F6ller =
[werk3AT]
>>Betreff: Re: [Springframework-developer] Hibernate resource management =
issue
>>
>>
>>
>>Juergen,
>>
>>I am almost 100% sure this block of code from
>>AbstractPlatformTransactionManager is wrong:
>>       else {
>>           try {
>>               try {
>>                   triggerBeforeCommit(defStatus);
>>                   triggerBeforeCompletion(defStatus);
>>                   if (status.isNewTransaction()) {
>>                       logger.info("Initiating transaction commit");
>>                       doCommit(defStatus);
>>                   }
>>               }
>>               catch (UnexpectedRollbackException ex) {
>>                   triggerAfterCompletion(defStatus,
>>TransactionSynchronization.STATUS_ROLLED_BACK, ex);
>>                   throw ex;
>>               }
>>               catch (TransactionException ex) {
>>                   if (this.rollbackOnCommitFailure) {
>>                       doRollbackOnCommitException(defStatus, ex);
>>                       triggerAfterCompletion(defStatus,
>>TransactionSynchronization.STATUS_ROLLED_BACK, ex);
>>                   }
>>                   else {
>>                       triggerAfterCompletion(defStatus,
>>TransactionSynchronization.STATUS_UNKNOWN, ex);
>>                   }
>>                   throw ex;
>>               }
>>               catch (RuntimeException ex) {
>>                   doRollbackOnCommitException(defStatus, ex);
>>                   triggerAfterCompletion(defStatus,
>>TransactionSynchronization.STATUS_ROLLED_BACK, ex);
>>                   throw ex;
>>               }
>>               catch (Error err) {
>>                   doRollbackOnCommitException(defStatus, err);
>>                   triggerAfterCompletion(defStatus,
>>TransactionSynchronization.STATUS_UNKNOWN, err);
>>                   throw err;
>>               }
>>               triggerAfterCompletion(defStatus,
>>TransactionSynchronization.STATUS_COMMITTED, null);
>>           }
>>           finally {
>>               cleanupAfterCompletion(defStatus);
>>           }
>>       }
>>
>>triggerBeforeCommit() execute, which in the Hibernate case will force a
>>flush. However, if that flush throws an exception, then
>>  triggerBeforeCompletion(defStatus);
>>never gets called. However, triggerBeforeCompletion is what is actually
>>supposed to release the Hibernate session holder from the current
>>thread! So in this case, the session (which is totally hosed of course)=
,
>>gets left on the thread. I believe in some environments this wouldn't
>>matter that much, as the threads don't get resused. In the JBoss case,
>>new requests coming in will get the existing thread, and this time,
>>SessionFactoryUtils will see the session is there, and try to use it.
>>Bang, it all blows up...
>>
>>So for this code to work properly, what needs to happen is that
>>triggerBeforeCompletion still needs to be called even if
>>triggerBeforeCommit fails. While I am ok with writing the code in this
>>method to handle this, I am not 100% sure this is safe in terms of all
>>the other interactions that will happen as a result; mot of this code i=
s
>>your baby with me only having traced through it once in a while. So if
>>you would prefer to resolve this that would be great.
>>
>>Unless I am mistaken about this bug, I think it is a pretty serious one=
,
>>and warrants an almost immediate release of a v1.0.1 of Spring...
>>
>>Regards,
>>Colin
>>
>>
>>Colin Sampaleanu wrote:
>>
>>
>>
>>   =20
>>
>>>I am tracking down a possible Hibernate resource management issue.
>>>
>>>In a running app, some time yesterday, some code, running in a wrapped
>>>transaction with Hibernate handling ORM, encountered an Oracle
>>>constraint violation and threw an exception. Fine...
>>>
>>>But when I log into the app myself now via the web ui and then it gets
>>>a service object to read some data, the service object is wrapped with
>>>a transaction interceptor, and also a hibernate interceptor. The
>>>Hibernate interceptor is already seeing a Hibernate Session existing
>>>on the current thread, so it is not creating a new one. Then at the
>>>end of the transaction, when the Hibernate session is attempted to be
>>>flushed, Hibernate tries to write out the old bad data from yesterday.
>>>
>>>What this essentially means is that when the error from yesterday
>>>happened, the session did not get released from the thread, and has
>>>been sticking around all this time. When I came via struts, I was
>>>given the same thread as yesterday by the appserver, and the old
>>>invalid session was still on it. The problem is not that it's reusing
>>>that session, but why it was ever left that the day before.
>>>
>>>Will try to duplicate this...
>>> =20
>>>
>>>     =20
>>>
>>
>>
>>
>>
>>   =20
>>
>
>
>
> =20
>