|
From: Colin S. <col...@ex...> - 2004-03-26 15:00:38
|
Well, I've never seen a case of the VM not calling catch and finally=20
blocks, so for the 2nd problem you have, so my feeling is that it's=20
almost certainly got to be a code issue, i.e. the code as written does=20
not to do exactly what you think it does... Could there be another=20
catch Throwable somewhere catching even the Error before it gets up to=20
that layer?
j=FCrgen h=F6ller [werk3AT] wrote:
>triggerAfterCompletion is now called by doRollbackOnCommitException (tha=
t the catch blocks delegate to), with the correct completion status. I've=
just added catch blocks for RuntimeException and Error there, calling tr=
iggerAfterCompletion too - although a rollback should never throw an exce=
ption other than TransactionException, so that's just to be on the safe s=
ide.
>
>Coincidentally, we're just debugging a ThreadLocal problem at werk3AT, b=
ut with HibernateTransactionManager: In case of some StackOverflowErrors =
thrown by data access operations, the catch/finally blocks of our transac=
tion management do not get called (though they catch Error), thus the Hib=
ernate 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 call=
ed by the VM... We're currently trying to reproduce this, as it just occu=
rs 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=
=20
>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=
=20
>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:
>
> =20
>
>>I see - if flush fails, the Session is closed (if no TransactionManager=
Lookup) but not removed from the thread. I've already fixed the issue; wi=
ll commit it promptly. Of course, none of this affects HibernateTransacti=
onManager (which is assumably the primary choice for Spring apps), but it=
's still a nasty issue.
>>
>>Bad timing - one day earlier, and we would simply have delayed 1.0 fina=
l for this. I agree that we should do a quick 1.0.1 followup: I've also i=
ncorporated more sophisticated FieldError message resolution, as suggeste=
d recently, and will also look at the reported auto-proxy creator issue w=
ith multiple afterPropertiesSet calls.
>>
>>All things considered, I suggest 1.0.1 mid next week. Let's also incorp=
orate 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 origina=
l
>>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 neve=
r
>>gets unbound from the thread. Nasty!
>>
>>Colin
>>
>>
>>j=FCrgen h=F6ller [werk3AT] wrote:
>>
>>=20
>>
>> =20
>>
>>>Odd - if there's no TransactionManagerLookup, the Session should defin=
itely 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 failu=
re.
>>>
>>>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 no=
t
>>>using TransactionManagerLookup... But in my case, because of the
>>>exception in the flush in beforeCommit(), beforeCompletion() never get=
s
>>>called (as it would normally). Now I do see that afterCompletion is al=
so
>>>supposed to call closeSessionIfNecessary() (I had actually missed thi=
s
>>>before), but in my case, it's not getting there. I haven't traced it i=
n
>>>a debugger, which is what I will do now, as it seems to me it should g=
et
>>>to that code, certainly I do have a
>>> "Triggering afterCompletion synchronization"
>>>in the log which should happen right before afterCompletion() is calle=
d.
>>>
>>>The other question is whether beforeCompletion still shouldn't be call=
ed
>>>in any case even if beforeCommit fails. Ultimately, we are still befor=
e
>>>completion of the transaction, unless you meant it to be only called i=
n
>>>the case of no failure.
>>>
>>>Colin
>>>
>>>
>>>j=FCrgen h=F6ller [werk3AT] wrote:
>>>
>>>
>>>
>>> =20
>>>
>>> =20
>>>
>>>>Colin,
>>>>
>>>>Thanks for tracking this down. It's actually a problem with SessionFa=
ctoryUtils' inner class SessionSynchronization: It assumes that beforeCom=
pletion is called in any case, even if beforeCommit has thrown an excepti=
on. However, this just applies if you specified a TransactionManagerLooku=
p in the Hibernate configuration; else, afterCompletion will do the clean=
up - 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 do=
n't need that TransactionManagerLookup when using Spring's JtaTransaction=
Manager, as Spring will properly apply cache callbacks anyway. A Transact=
ionManagerLookup just adds value when used with EJB CMT or manual JTA, fo=
r ultra-correct cache callbacks.
>>>>
>>>>So essentially, everything should be fine if using HibernateTransacti=
onManager, or JtaTransactionManager without a Hibernate TransactionManage=
rLookup. This is clearly something to fix, but I guess we don't need to d=
o an immediate 1.0.1 followup release; I'd like to gather further bug rep=
orts first. For the time being, let's suggest to remove the TransactionMa=
nagerLookup from the Hibernate configuration.
>>>>
>>>>Juergen
>>>>
>>>>
>>>>________________________________
>>>>
>>>>Von: Colin Sampaleanu [mailto:col...@ex...]
>>>>Gesendet: Do 25.03.2004 20:32
>>>>An: spr...@li...; j=FCrgen h=F6lle=
r [werk3AT]
>>>>Betreff: Re: [Springframework-developer] Hibernate resource managemen=
t 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 actual=
ly
>>>>supposed to release the Hibernate session holder from the current
>>>>thread! So in this case, the session (which is totally hosed of cours=
e),
>>>>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 thi=
s
>>>>method to handle this, I am not 100% sure this is safe in terms of al=
l
>>>>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 i=
f
>>>>you would prefer to resolve this that would be great.
>>>>
>>>>Unless I am mistaken about this bug, I think it is a pretty serious o=
ne,
>>>>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 wrapp=
ed
>>>>>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 ge=
ts
>>>>>a service object to read some data, the service object is wrapped wi=
th
>>>>>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 b=
e
>>>>>flushed, Hibernate tries to write out the old bad data from yesterda=
y.
>>>>>
>>>>>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 reusin=
g
>>>>>that session, but why it was ever left that the day before.
>>>>>
>>>>>Will try to duplicate this...
>>>>> =20
>>>>>
>>>>> =20
>>>>>
>
>
> =20
>
|