Automatically started UnitOfWork is never ended

[gissuebot]

From spootsy.ootsy on September 26, 2012 22:48:16

We've been using Guice in our projects for a while now, and it's been great. There's one feature/bug that is a reliable source of bugs for us, though.

When using the JpaPersistService, if you attempt to access an EntityManager outside of an active UnitOfWork, Guice will automatically start one for you. However, as Guice does not (and cannot) know when to end this UnitOfWork, it never does.

Result? The offending thread will be stuck with the same EntityManager throughout the life of the application. This is a bad state for the application to run in, and ours inevitably exhaust the available memory after a while and crash.

The real killer here is that it's not at all obvious when you've made this mistake. The only real tip-off is that you're getting inconsistent data from your database between different threads (due to the EMs first-level cache) or that the applications memory consumption keeps on growing.

I think it would be much better behaviour to simply throw an exception if an attempt is made to access an EntityManager instance outside of an active UnitOfWork. This lets the developer know that they've made a mistake and are attempting to access the database outside of a valid scope. Steps to reproduce: 1. 'mvn test' on the supplied maven project (assumes postgres running on localhost:5432 with DB/user/pass 'test')

Binary attachments: guice_persist_same_em.zip

Original issue: http://code.google.com/p/google-guice/issues/detail?id=730

[gissuebot]

From spootsy.ootsy on October 10, 2012 16:44:26

I've created a git patch which addresses this issue, as well as issue 731 . Rundown of changes from the commit log:

• JpaPersistService now counts invocations of begin() and end(), and only closes
    EntityManager when end() has been called an appropriate number of times
• Attempting to get an EntityManager outside of an active UnitOfWork now results
    in an IllegalStateException
• Added a new @WorkUnit annotation. This is equivalent to the @Transactional
    annotation for methods/classes which access the database but do not modify it

Patch also updates the appropriate unit tests.

Attachment: gist
   0001-Updates-to-JpaPersistService-implementation-of-UnitO.patch

[gissuebot]

From prashant.mr on October 10, 2012 18:54:55

@WorkUnit annotation, is this similar to Spring's @Transactional(readOnly = true) http://static.springsource.org/spring/docs/3.1.x/spring-framework-reference/html/transaction.html#transaction-declarative-annotations http://static.springsource.org/spring/docs/3.1.x/javadoc-api/org/springframework/transaction/annotation/Transactional.html#readOnly() IMO, readOnly=true expresses the transaction's behavior clearly that the changes will not be committed.

[gissuebot]

From spootsy.ootsy on October 10, 2012 20:11:22

@WorkUnit is just a shorthand means of using the UnitOfWork interface, by wrapping the annotated method with calls to uow.begin/uow.end in a try/finally block. No JPA transaction is explicitly started.

I think it's conceptually different from a read-only transaction. That would certainly make more sense as an additional attribute on the existing @Transactional annotation.

[gissuebot]

From bimschas on August 22, 2013 14:38:05

Will this fix be part of Guice 4.0?

[gissuebot]

From spootsy.ootsy on August 22, 2013 22:01:52

I was hoping they'd include a patch of some kind but I don't see any changes in trunk. It's a shame, because the current semantics of UnitOfWork are pretty badly broken IMHO.

Also, please note there's a significant bug in this patch, as discussed here: https://code.google.com/p/google-guice/issues/detail?id=731 It's a simple fix but I haven't got around to it yet. If anybody else has a vested interest you're more than welcome to fix it and post the updated patch here.

[gissuebot]

From sberlin on December 20, 2013 06:17:02

(No comment was entered for this change.)

Labels: Component-Persist

[woru]

+1 IMHO this a serious bug. You forget to add @transactional and end up with OutOfMemoryError in production.
Also JpaPersistService is not public and cannot be overridden.