Menu
▾
▴
Re: [Springframework-developer] Hibernate resource management issue
|
From: Colin S. <col...@ex...> - 2004-03-26 12:50:52
|
I see you also changed the behaviour so that on a RuntimeException or
Error, triggerAfterCompletion does not get called any longer, whereas it
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 not
100% clear on the semantics of triggerAfterCompletion and whether it
always has to be called, I presume it does).
jürgen höller [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.
>
>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.
>
>All things considered, I suggest 1.0.1 mid next week. Let's also incorporate all reported documentation inconsistencies and the like.
>
>Juergen
>
>
>________________________________
>
>Von: Colin Sampaleanu [mailto:col...@ex...]
>Gesendet: Do 25.03.2004 21:36
>An: jürgen höller [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) {
>
>TransactionSynchronizationManager.unbindResource(this.sessionFactory);
> if (this.hibernateTransactionCompletion) {
>
>closeSessionIfNecessary(this.sessionHolder.getSession(),
>this.sessionFactory);
> }
> }
> }
>
> public void afterCompletion(int status) {
> if (!this.hibernateTransactionCompletion) {
> Session session = this.sessionHolder.getSession();
> if (session instanceof SessionImplementor) {
> ((SessionImplementor)
>session).afterTransactionCompletion(status == 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ürgen höller [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ürgen höller [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ürgen höller [werk3AT] wrote:
>>
>>
>>
>>
>>
>>>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ürgen höller [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:
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>>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...
>>>>
>>>>
|
