Re: [Springframework-developer] Hibernate resource management issue

[<jue...@we...>]

I see - if flush fails, the Session is closed (if no =
TransactionManagerLookup) but not removed from the thread. I've already =
fixed the issue; will commit it promptly. Of course, none of this =
affects HibernateTransactionManager (which is assumably the primary =
choice for Spring apps), but it's still a nasty issue.
=20
Bad timing - one day earlier, and we would simply have delayed 1.0 final =
for this. I agree that we should do a quick 1.0.1 followup: I've also =
incorporated more sophisticated FieldError message resolution, as =
suggested recently, and will also look at the reported auto-proxy =
creator issue with multiple afterPropertiesSet calls.
=20
All things considered, I suggest 1.0.1 mid next week. Let's also =
incorporate all reported documentation inconsistencies and the like.
=20
Juergen
=20

________________________________

Von: Colin Sampaleanu [mailto:col...@ex...]
Gesendet: Do 25.03.2004 21:36
An: j=FCrgen h=F6ller [werk3AT]
Cc: spr...@li...
Betreff: Re: [Springframework-developer] Hibernate resource management =
issue



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

        public void afterCompletion(int status) {
            if (!this.hibernateTransactionCompletion) {
                Session session =3D this.sessionHolder.getSession();
                if (session instanceof SessionImplementor) {
                    ((SessionImplementor)
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
never gets called.

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

Colin


j=FCrgen h=F6ller [werk3AT] wrote:

>Odd - if there's no TransactionManagerLookup, the Session should =
definitely be closed in afterCompletion.
>
>Regarding beforeCompletion, the semantics indeed need to be clarified: =
I guess it's appropriate to always invoke it, even on beforeCommit =
failure.
>
>Juergen
>
>
>________________________________
>
>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 =
issue
>
>
>
>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 =
SessionFactoryUtils' inner class SessionSynchronization: It assumes that =
beforeCompletion 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 =
face the issue - the bug doesn't have any effects then. Note that you =
don't need that TransactionManagerLookup when using Spring's =
JtaTransactionManager, as Spring will properly apply cache callbacks =
anyway. A TransactionManagerLookup just adds value when used with EJB =
CMT or manual JTA, for ultra-correct cache callbacks.
>>
>>So essentially, everything should be fine if using =
HibernateTransactionManager, or JtaTransactionManager without a =
Hibernate TransactionManagerLookup. 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 reports first. For the time being, let's =
suggest to remove the TransactionManagerLookup 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 =
is
>>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
>