JPA EntityManager Static or Instance?

Question

I've built some JPA stuff in the past that used an instance of the javax.persistence.EntityManager per instance of DAO; this is the way most examples are setup.

public class BaseDaoThatEveryDaoExtends {
   @PersistenceContext
   private EntityManager entityManager;
}

I've just stumbled upon code that uses a static javax.peristence.EntityManger injected by a PersistenceContext annotation, the architect tells me this causes no problems and they never had any problem even in a clustered application with JTA and an XA data source:

public class BaseDaoThatEveryDaoExtends {
   @PersistenceContext
   private static EntityManager entityManager;
}

As far as I can tell this is an anti-pattern as the EntityManager holds some state information and making it static makes that whole state application wide. Also this makes the classes very hard to test.

Are there other drawbacks to doing this or is this a standard way of using an EntityManager ?

Answer 1

I think the major risk is not in the EntityManager itself, but in the application context that is attached to the entity manager when it is used.

Let's assume you have two different clients making a request to your server, both invoking two different methods of your application, both running on different threads, but both using the same entity manager.

As far as I know the entity manager will have a single context attached to it, this context will be shared by both clients. Every time you load an instance to the context, will be available to threads through the shared entity manager. What will happen if they tamper with each other's data? What will happen if they are using different transaction isolation configuration?. How can you possibly be sure that client 1 is not altering data being currently used by client 2?

What if one of the clients invalidates the context, what will the other do? How do you deal with concurrency this way?

Answer 2

EntityManager holds onto its data using a threadlocal, so it may be ok to hold a static reference to it since all the threads accessing it would be handled independently. In fact, I would not be surprised if the EJB context holds onto the EntityManager in a static way, using a singleton pattern.

Personally, I would never define it in a static way. It seems unnecessary, and at worst may have some unforeseen side effects.

One problem I can see is the ability to inadvertently accessing the entityManager from a static method:

public class BaseDaoThatEveryDaoExtends {
   @PersistenceContext
   private static EntityManager entityManager;

   public static void doSomeStaticWork(){
      ...
      entityManager.doSomething; //NPE possible!
   }
}

I could see the EntityManager not being injected and resulting in a NPE in this case.

Other than that there may be some issues around testing/mocking using the EntityManager.

Answer 3

The EntityManagerFactory is guaranteed to be thread-safe, so I think this is the "correct" way: Use the EMF in the thread-un-safe places and protect the EntityManger itself from threading issues.