Menu
▾
▴
Re: [Springframework-developer] Hibernate resource management issue
|
From: Colin S. <col...@ex...> - 2004-03-26 22:20:08
|
We should probably wrap all logging calls in critical sections (like=20
catch blocks) in the tx and thread resource mgmt code with empty=20
try/catch blocks, to avoid this sort of thing in the future.
j=FCrgen h=F6ller [werk3AT] wrote:
>The bug was pretty obscure: It was caused by an exception that came acro=
ss the wire via Hessian. (We had to track down that scenario first.) The =
exception was a custom subclass of Spring's NestedRuntimeException. For w=
hatever reason, Hessian put a reference to the exception itself into the =
"cause" instance variable. (This situation cannot occur through normal us=
age of NestedRuntimeException, as it just allows to specify the cause in =
a constructor.) This caused NestedRuntimeException's getMessage and print=
StackTrace to recurse indefinitely, resulting in a StackOverflowError. As=
this was happening in logging code within a catch block of TransactionIn=
terceptor, rollback was never invoked, thus no cleanup of the associated =
transactional resources.
>=20
>A candidate for the most obscure bug in quite a while...
>=20
>Juergen
>=20
>
>________________________________
>
>Von: spr...@li... im Auftrag vo=
n Colin Sampaleanu
>Gesendet: Fr 26.03.2004 16:00
>An: spr...@li...
>Betreff: Re: [Springframework-developer] Hibernate resource management i=
ssue
>
>
>
>Well, I've never seen a case of the VM not calling catch and finally
>blocks, so for the 2nd problem you have, so my feeling is that it's
>almost certainly got to be a code issue, i.e. the code as written does
>not to do exactly what you think it does... Could there be another
>catch Throwable somewhere catching even the Error before it gets up to
>that layer?
>
>j=FCrgen h=F6ller [werk3AT] wrote:
>
> =20
>
>>triggerAfterCompletion is now called by doRollbackOnCommitException (th=
at the catch blocks delegate to), with the correct completion status. I'v=
e just added catch blocks for RuntimeException and Error there, calling t=
riggerAfterCompletion too - although a rollback should never throw an exc=
eption 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 transa=
ction management do not get called (though they catch Error), thus the Hi=
bernate Session stays attached to the thread. I'm not sure if I understan=
d this: Even in case of an Error, catch and finally blocks should get cal=
led by the VM... We're currently trying to reproduce this, as it just occ=
urs 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
>>Error, triggerAfterCompletion does not get called any longer, whereas i=
t
>>did with the previous code. I am not 100% sure why this went away,
>>considering it was there before, and it is still being done for
>>UnexpectedRollbackException and TransactionException. (However, I'm no=
t
>>100% clear on the semantics of triggerAfterCompletion and whether it
>>always has to be called, I presume it does).
>>
>>
>>j=FCrgen h=F6ller [werk3AT] wrote:
>>
>>
>>
>> =20
>>
>>>I see - if flush fails, the Session is closed (if no TransactionManage=
rLookup) but not removed from the thread. I've already fixed the issue; w=
ill commit it promptly. Of course, none of this affects HibernateTransact=
ionManager (which is assumably the primary choice for Spring apps), but i=
t's still a nasty issue.
>>>
>>>Bad timing - one day earlier, and we would simply have delayed 1.0 fin=
al for this. I agree that we should do a quick 1.0.1 followup: I've also =
incorporated more sophisticated FieldError message resolution, as suggest=
ed recently, and will also look at the reported auto-proxy creator issue =
with multiple afterPropertiesSet calls.
>>>
>>>All things considered, I suggest 1.0.1 mid next week. Let's also incor=
porate all reported documentation inconsistencies and the like.
>>>
>>>Juergen
>>>
>>>
>>>________________________________
>>>
>>>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 origin=
al
>>>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 nev=
er
>>>gets unbound from the thread. Nasty!
>>>
>>>Colin
>>>
>>>
>>>j=FCrgen h=F6ller [werk3AT] wrote:
>>>
>>>
>>>
>>> =20
>>>
>>> =20
>>>
>>>>Odd - if there's no TransactionManagerLookup, the Session should defi=
nitely 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 fail=
ure.
>>>>
>>>>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 managemen=
t issue
>>>>
>>>>
>>>>
>>>>We're on slightly different pages though. In my case, I am actually n=
ot
>>>>using TransactionManagerLookup... But in my case, because of the
>>>>exception in the flush in beforeCommit(), beforeCompletion() never ge=
ts
>>>>called (as it would normally). Now I do see that afterCompletion is a=
lso
>>>>supposed to call closeSessionIfNecessary() (I had actually missed th=
is
>>>>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 call=
ed.
>>>>
>>>>The other question is whether beforeCompletion still shouldn't be cal=
led
>>>>in any case even if beforeCommit fails. Ultimately, we are still befo=
re
>>>>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
>>>>
>>>> =20
>>>>
>>>> =20
>>>>
>>>>>Colin,
>>>>>
>>>>>Thanks for tracking this down. It's actually a problem with SessionF=
actoryUtils' inner class SessionSynchronization: It assumes that beforeCo=
mpletion is called in any case, even if beforeCommit has thrown an except=
ion. However, this just applies if you specified a TransactionManagerLook=
up in the Hibernate configuration; else, afterCompletion will do the clea=
nup - 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 d=
on't need that TransactionManagerLookup when using Spring's JtaTransactio=
nManager, as Spring will properly apply cache callbacks anyway. A Transac=
tionManagerLookup just adds value when used with EJB CMT or manual JTA, f=
or ultra-correct cache callbacks.
>>>>>
>>>>>So essentially, everything should be fine if using HibernateTransact=
ionManager, or JtaTransactionManager without a Hibernate TransactionManag=
erLookup. 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 re=
ports first. For the time being, let's suggest to remove the TransactionM=
anagerLookup from the Hibernate configuration.
>>>>>
>>>>>Juergen
>>>>>
>>>>>
>>>>>________________________________
>>>>>
>>>>>Von: Colin Sampaleanu [mailto:col...@ex...]
>>>>>Gesendet: Do 25.03.2004 20:32
>>>>>An: spr...@li...; j=FCrgen h=F6ll=
er [werk3AT]
>>>>>Betreff: Re: [Springframework-developer] Hibernate resource manageme=
nt 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 forc=
e a
>>>>>flush. However, if that flush throws an exception, then
>>>>>triggerBeforeCompletion(defStatus);
>>>>>never gets called. However, triggerBeforeCompletion is what is actua=
lly
>>>>>supposed to release the Hibernate session holder from the current
>>>>>thread! So in this case, the session (which is totally hosed of cour=
se),
>>>>>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 cas=
e,
>>>>>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 th=
is
>>>>>method to handle this, I am not 100% sure this is safe in terms of a=
ll
>>>>>the other interactions that will happen as a result; mot of this cod=
e 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
>>>>>
>>>>> =20
>>>>>
>>>>>>I am tracking down a possible Hibernate resource management issue.
>>>>>>
>>>>>>In a running app, some time yesterday, some code, running in a wrap=
ped
>>>>>>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 g=
ets
>>>>>>a service object to read some data, the service object is wrapped w=
ith
>>>>>>a transaction interceptor, and also a hibernate interceptor. The
>>>>>>Hibernate interceptor is already seeing a Hibernate Session existin=
g
>>>>>>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 yesterd=
ay.
>>>>>>
>>>>>>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 reusi=
ng
>>>>>>that session, but why it was ever left that the day before.
>>>>>>
>>>>>>Will try to duplicate this...
>>>>>> =20
>>>>>>
|
