Is using synchronized on a Java DAO going to cause issues?

Question

Is using the 'synchronized' keyword on methods in a Java DAO going to cause issues when used by a web application?

I ask because I have a multi-threaded stand alone application that needs the methods to by synchronized to avoid resource conflict, as seen here.

java.util.concurrent.ExecutionException: javax.persistence.PersistenceException: org.hibernate.HibernateException: Found shared references to a collection: com.replaced.orm.jpa.Entity.stuffCollection

What I am concerned about is that when a significant number of people try and use the application that the synchronized methods will block and slow the entire application down.

I am using a Spring injected JPA entity manager factory, which provides an entity manager to the DAO. I could technically remove the DAO layer and have the classes call the entity manager factory directly, but I enjoy the separation the DAO provides.

I should also note that I am being very careful not to pass around connected entity ORM objects between threads. I speculate that the resource conflict error comes about when accessing the DAO. I think multiple threads are going at the same time and try to persist or read from the database in non-atomic ways.

In this case is using a DAO going to do more harm then help?

---

A big piece of information I left out of the question is that the DAO is not a singleton. If I had been thinking lucidly enough to include that detail I probably wouldn't have asked the question in the first place.

If I understand correctly, Spring creates a new instance of the DAO class for each class that uses it. So the backing entity manager should be unique to each thread. Not sharing the entity manager is, as Rob H answered, the key thing here.

However, now I don't understand why I get errors when I remove synchronized.

---

According to this thread, the @PersistenceContext annotation creates a thread-safe SharedEntityManager. So you should be able to create a singleton DAO.

Answer 1

You say you are not sharing entity objects across threads. That's good. But you should also make sure you're not sharing EntityManager objects (or Session objects in Hibernate) across threads either. Frameworks like Spring manage this for you automatically by storing the session in a thread-local variable. If you're coding your own DAOs without the help of a framework, you need to take precautions yourself to avoid sharing them.

Once you do this, there should be no reason to synchronize DAO methods because none of the conversational state will be shared across threads. This is critical for a highly concurrent web application. The alternative is that only one thread will be able to access the DAO at one time, assuming they all share the same DAO instance. Not good at all for throughput.

Answer 2

If it needs to be synchronized for thread safety, then leave them there. The blocking is required anyway in that case. If the blocking is not required for the web application case, you can either:

• leave it as is, since the performance hit when there is no contention on the lock is negligible, and insignificant when taken into account the expense of hitting the database.
• Redesign it so that you add a synchronization layer for the standalone application case which protects the underlying unsynchronized DAO.

Personally, I would leave it as is and profile it to see if you need to refactor. Until then you are simply doing premature optimization.