[hibernate-issues] [Hibernate-JIRA] Commented: (HHH-4784) JDBCTransaction -> commit() -> notifyLocalSynchsBeforeTransactionCompletion()

[Silvan Eugen Lincan (JIRA)]

    [ http://opensource.atlassian.com/projects/hibernate/browse/HHH-4784?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=35783#action_35783 ] 

Silvan Eugen Lincan commented on HHH-4784:
------------------------------------------

Hi Adam, thank you very much for your answer.

Yes, this is were I wanted to arrive.

The code in AuditSync.beforCompletion() goes hand in hand with the code from JDBCTransaction.notifyLocalSynchsBeforeTransactionCompletion().
If we correct the code here, like I said in my first "Description"(or maybe there is a more good way), then you should never do transaction rollback in Envers. You just have to throw the exception.

In my opinion, you have to let the upper layer to take care of transaction management, you don't have to manage it.

And also, this is related to first comment on HHH-3543. I had the same problem, because I have flush() set on MANUAL, this case is treated as a separate one in the same method in AuditSync.beforeCompletion().
You open a temporary non-transactional session which will not enrol in the upper one, and at the end (if something goes wrong in Envers), you will end up with the modifications from hibernate committed to database but with Envers tables empty, this should not happen, all transaction should be rolled back.

I don't know if I was clear here in what I have written. Don't hesitate to ask.
I have corrected this on my version on 3.5.0 CR1 hibernate distribution.
If you want, I can provide a patch for this.
With this patch, all hibernate tests pass, and all Envers tests(with one exception, wich I didn't have time to fix). I will tell you more if you want.

> JDBCTransaction -> commit() -> notifyLocalSynchsBeforeTransactionCompletion()
> -----------------------------------------------------------------------------
>
>                 Key: HHH-4784
>                 URL: http://opensource.atlassian.com/projects/hibernate/browse/HHH-4784
>             Project: Hibernate Core
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 3.5.0-Beta-2
>         Environment: hibernate-distribution-3.5.0-Beta-2
> ms-sqlserver 2008
>            Reporter: Silvan Eugen Lincan
>            Assignee: Adam Warski
>             Fix For: 3.5.0.Next
>
>
> Java class: org.hibernate.transaction.JDBCTransaction
> method: commit()
> line: 140 call to notifyLocalSynchsBeforeTransactionCompletion()
> The problem is this: method notifyLocalSynchsBeforeTransactionCompletion(), it swallows any exception that may occur at this level and permits the execution flow of commit() method to continue.
> But in this case, the hibernate session may be already corrupted, so the commit will be wrong.
> In my opinion, the method notifyLocalSynchsBeforeTransactionCompletion() should throw the exception, and on line 140 in commit() method, the call to this notifyLocalSynchsBeforeTransactionCompletion() should be surrounded also in the try{}catch(){}.
> Otherwise, any error that can occur on Synchronization, will be omitted and the transaction committed(it should be rolled back).
> I had this problem, executing the tests from envers module with the ms sqlserver 2008 database.
> The test org.hibernate.envers.test.integration.reventity.ExceptionListener crashes.
> I hope that it deserves your attention.
> Thank you very much.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://opensource.atlassian.com/projects/hibernate/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira