Menu
▾
▴
Re: [Springframework-developer] Hibernate resource management issue
|
From: <jue...@we...> - 2004-03-26 14:39:14
|
triggerAfterCompletion is now called by doRollbackOnCommitException =
(that the catch blocks delegate to), with the correct completion status. =
I've just added catch blocks for RuntimeException and Error there, =
calling triggerAfterCompletion too - although a rollback should never =
throw an exception other than TransactionException, so that's just to be =
on the safe side.
Coincidentally, we're just debugging a ThreadLocal problem at werk3AT, =
but with HibernateTransactionManager: In case of some =
StackOverflowErrors thrown by data access operations, the catch/finally =
blocks of our transaction management do not get called (though they =
catch Error), thus the Hibernate Session stays attached to the thread. =
I'm not sure if I understand this: Even in case of an Error, catch and =
finally blocks should get called by the VM... We're currently trying to =
reproduce this, as it just occurs on some machines and just in special =
scenarios.
Juergen
-----Original Message-----
From: Colin Sampaleanu [mailto:col...@ex...]
Sent: Friday, March 26, 2004 1:54 PM
To: spr...@li...; j=FCrgen h=F6ller
[werk3AT]
Subject: Re: [Springframework-developer] Hibernate resource management
issue
I see you also changed the behaviour so that on a RuntimeException or=20
Error, triggerAfterCompletion does not get called any longer, whereas it =
did with the previous code. I am not 100% sure why this went away,=20
considering it was there before, and it is still being done for=20
UnexpectedRollbackException and TransactionException. (However, I'm not =
100% clear on the semantics of triggerAfterCompletion and whether it=20
always has to be called, I presume it does).
j=FCrgen h=F6ller [werk3AT] wrote:
>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:
>
> =20
>
>>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
>>>
>>> =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
>>>>
|
