Can spring transactions unsynchronize a synchronized method?

Question

My colleague and I have a web application that uses Spring 3.0.0 and JPA (hibernate 3.5.0-Beta2) on Tomcat inside MyEclipse. One of the data structures is a tree. Just for fun, we tried stress-testing the "insert node" operation with JMeter, and found a concurrency problem. Hibernate reports finding two entities with the same private key, just after a warning like this:

 WARN  [org.hibernate.engine.loading.LoadContexts] fail-safe cleanup (collections) : ...

It is quite easy to see how such problems might occur if multiple threads call the insert() method concurrently.

My servlet A calls a service layer object B.execute(), which then calls a lower layer object C.insert(). (The real code is too big to post, so this is somewhat abridged.)

Servlet A:

  public void doPost(Request request, Response response) {
    ...
    b.execute(parameters);
    ...
  }

Service B:

  @Transactional //** Delete this line to fix the problem.
  public synchronized void execute(parameters) {
    log("b.execute() starting. This="+this);
    ...
    c.insert(params);
    ...
    log("b.execute() finishing. This="+this);
  }

Sub-service C:

  @Transactional
  public void insert(params) {
    ...
    // data structure manipulation operations that should not be 
    // simultaneous with any other manipulation operations called by B.
    ...
  }

All my state-change calls go through B, so I decided to make B.execute() synchronized. It was already @Transactional, but it's actually the business logic that needs to be synchronized, not just the persistence, so that seems reasonable.

My C.insert() method was also @Transactional. But since the default transaction propagation in Spring seems to be Required, I don't think there was any new transaction being created for C.insert().

All components A, B, and C are spring-beans, and hence singletons. If there really is only one B object, then I conclude that it shouldn't be possible for more than one threat to execute b.execute() at a time. When the load is light, only a single thread is used, and this is the case. But under load, additional threads get involved, and I see several threads print "starting" before the first one prints "finishing". This seems to be a violation of the synchronized nature of the method.

I decided to print the this in the log messages to confirm whether there was only a single B object. All the log messages do show the same object id.

After much frustrating investigation, I discovered that removing the @Transactional for B.execute() solves the problem. With that line gone, I can have lots of threads, but I always see a "starting" followed by a "finishing" before the next "starting" (and my data structures stay intact). Somehow, the synchronized only seems to work when the @Transactional is not present. But I don't understand why. Can anyone help? Any tips as to how to look into this further?

In stack traces, I can see that there's an aop/cglib proxy generated in between A.doPost() and B.execute() - and also between B.execute() and C.insert(). I wonder whether somehow the proxy's construction might ruin the synchronized behaviour.

Answer 1

The problem is that @Transactional encapsulates the synchronized method. Spring does this using AOP. The execution goes something like this:

1. start transaction
2. call the method annotated with @Transactional
3. when method returns commit the transaction

Steps 1. and 3. can be executed by many threads at the same time. Hence you get multiple starts of transaction.

Your only solution is to synchronize the call to the method itself.

Answer 2

Synchronized keyword requires, as you stated, that the object involved is always the same. I haven't observed the above mentioned behaviour myself but your suspect might just be correct one.

Have you tried logging out b from doPost -method? If that is different every time, then there is some spring magic with AOP/cglib proxies going on.

Anyway, I wouldn't rely on the syncronized keyword but use something like ReentrantLock from java.util.concurrent.locks to ensure the synchronization behavior instead, as your b object is always same regardless of possible multiple cglib proxies.